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