Fighting Bull
Self-defence class
With sensei Bodacious
1st degree black belt
Sensei Bodacious
(Gavin)
GitHub/bodacious
Slack/gavin
Twitter/morriceGavin
The biggest threat to you and your code...
- Other engineers
- Your squad-mates
- You
State burglar
(Direct field access)
Direct field access
class Order
attr_accessor :order_number
attr_accessor :items
end
Direct field access
@order = Order.new
@order.order_number = nil
@order.items = { foo: 'bar' }
Direct field access
- You don't have control over input values
- Can't ensure valid state (invariants)
- You can't guarantee the output values
Direct field access
Code should express intent, but not implementation
class Order
# attr_accessor :order_number
attr_accessor :items
def generate_order_number
@order_number = SecureRandom.hex(4)
end
def order_number
@order_number
end
end
Direct field access
class Order
# attr_accessor :order_number
# attr_accessor :items
def items
@items
end
def add_item(item)
@items << item
end
def generate_order_number
@order_number = SecureRandom.hex(4)
end
def order_number
@order_number
end
end
Pick-pocket
(State mutation)
State mutation
class Order
def initialize(items: [])
@items = items.to_a
@order_number = generate_unique_number
end
def items
@items
end
def order_number
@order_number
end
private
def generate_unique_number
@order_number = SecureRandom.hex(4)
end
end
State mutation
@order = Order.new(items: Item.new(sku: "shoes-123"))
@order.items << Item.new(sku: "aviators-456")
@order.order_number.clear
@order.order_number # => ""State mutation
- Consumers can still access state indirectly
- Can mutate state, without your (or their) knowledge
- Very difficult to detect and debug
State mutation
Internal state should be completely shielded from the external environment
class Order
def initialize(items: [])
@items = items.to_a
@order_number = generate_unique_number
end
def items
@items.dup
end
def order_number
@order_number.freeze
end
private
def generate_unique_number
@order_number = SecureRandom.hex(4)
end
end
The Entangler
(Leaky abstraction)
Leaky abstraction
class Order
def initialize(items: [])
@items = items.to_a
@order_number = generate_unique_number
@status = :pending_confirmation
self.total_price = 0
end
def total_price
@total_price
end
def total_price=(value)
@total_price = Money.new(value.to_i)
end
def status
@status
end
# ... same
end
class OrdersController < ApplicationController
def confirm
# ...
if @order.status == :pending_confirmation
@order.status = :confirmed
@order.items.each { |item| item.hold! }
@order.total_price = @order.items.sum(&:price)
end
# ...
end
end
Leaky abstraction
The consumer knows too much about the abstraction "confirm"
- Knowledge of how to do it is not concretely defined
- Easier to introduce bugs through error
- Makes change more risky and expensive
Leaky abstraction
class OrderController < ApplicationController
def confirm
# ...
if @order.status == :pending_confirmation
# @order.status = :confirmed
@order.items.each { |item| item.hold! }
@order.total_price = @order.items.sum(&:price)
end
# ...
end
end
Leaky abstraction
The consumer knows too much about the abstraction "confirm"
- Knowledge of how to do it is not concretely defined
- Easier to introduce bugs through error
- Makes change more risky and expensive
Leaky abstraction
class Order
def initialize(items: [])
@items = items.to_a
@order_number = generate_unique_number
@status = :pending_confirmation
self.total_price = 0
end
def may_confirm?
@status == :pending_confirmation
end
def confirm!
@status = :confirmed
@items.each(&:hold!)
self.total_price = items.sum(&:price)
end
def total_price
@total_price
end
def total_price=(value)
@total_price = Money.new(value.to_i)
end
# ... same
end
Behaviour should be internal and explicit, not implicit
Leaky abstraction
class OrderController < ApplicationController
def confirm
# ...
@order.confirm! if @order.may_confirm?
# ...
end
end
Leaky abstraction
class Order
def initialize(items: [])
@items = items.to_a
@order_number = generate_unique_number
@status = :pending_confirmation
@total_price = 0
end
def may_confirm?
@status == :pending_confirmation
end
private :may_confirm?
def confirm!
return false unless may_confirm?
@status = :confirmed
@items.each(&:hold!)
self.total_price = items.sum(&:price)
end
def total_price
@total_price
end
def total_price=(value)
@total_price = Money.new(value.to_i)
end
# ... same
end
Leaky abstraction
class OrderController < ApplicationController
def confirm
# ...
if @order.confirm!
# ...
end
end
Psychic mind
invader
(Private bypassing)
Private bypassing
class Order
def initialize(items: [])
@items = items
@order_number = generate_unique_number
@status = :pending
end
private
def generate_unique_number
# ...
end
end
Private bypassing
it sucks because ...
Private bypassing
@order = Order.new
@order.instance_variable_set(:@status, :cancelled)
@order.send(:generate_unique_number)
Private bypassing
- You cannot escape
- Just too powerful
- You should never need to use these!
Private bypassing
class Order
def initialize(items: [])
@items = items
@order_number = generate_unique_number
@status = :pending
end
undef :instance_variable_set,
:instance_variable_get,
:instance_variables,
:send
# ...
end
Private bypassing
@order = Order.new
@order.cancel!
@order.public_send(:generate_unique_number)
Enforce public interface methods at all times
Recap
Prefer methods over attribute setters
Code should express intent, but not implementation
Ensure exposed state is immutable
Internal state should be completely shielded from the external environment
Hide the inner workings behind an interface
Behaviour should be internal and explicit, not implicit
Avoid methods that allow direct access to hidden information
Sometimes the best defence is a good pair of running shoes
Questions?
Slides
Fighting Bull Self-defence
By Gavin Morrice
