Notes of 'My issues with Modules' by Ryan Bates. Source: https://gist.github.com/4172391

issue_with_modules.md

When working through unknown code method, ask:

What does a given method do?
What is the current object context that I am in?
What other methods can I call here?

module Purchasable
  def purchase
    result = Braintree::Transaction.sale(amount: total, credit_card: card)
    if result.success?
      self.purchased_at = Time.zone.now
      save!
    end
  end
end

What does a given method do?

It calls sale on a BrainTree::Transaction object with total (1) and card (2) methods. Then it cals purchased_at= (3) and save! (4). A total of 4 implicit method calls. Whichever object includes this module will need to implement these four methods.

What is the current object context that I am in?

Don't know, depends on what class includes this module.

Modules can be great for adding behavior if it isn't tightly coupled to the class that's including it.

module Purchasable
  def purchase(total, card)
    result = Braintree::Transaction.sale(amount: total, credit_card: card)
    result.success? # leave it up to the caller to mark as purchased
  end
end

This leads to the problem of too many modules being included in the class. Another issue is conflicting methods. For example ActiveRecord::Validations overrides save method to add validations. If you want to find the source of save, you'll need to look in two places now.

Instead create a class:

class Purchase
  def initialize(order)
    @order = order
  end

  def submit
    result = Braintree::Transaction.sale(amount: @order.total, credit_card: @order.card)
    if result.success?
      @order.mark_as_purchased!
    end
  end
end

What does a given method do? It's easy enough to open the Order class and find out.
What is the current object context that I am in? A Purchase instance
What other methods can I call here? Anything defined on Purchase

Now Purchase is tied to order. A better interface is:

class Purchase
  def initialize(amount, credit_card)
    @amount = amount
    @credit_card = credit_card
  end

  def submit
    result = Braintree::Transaction.sale(amount: @amount, credit_card: @credit_card)
    result.success?
  end
end

# use it like
class OrderController
  def create
    @order = Order.new
    purchase = Purchase.new(@order.amount, @order.credit_card)
  
    if purchase.submit
      @order.mark_as_purchased!
    else
      # handle errors
    end
  end
end

In above example Purchase and Order are loosely coupled, however OrderController is now tightly coupled to Order and Purchase. If it wasn't a Rails controller, I'd inject Order and Purchase.

class OrderController
  attr_reader :order, :purchase
  
  def initialize(order, purchase)
    @order = order
    @purchase = purchase
  end
  
  def run
    if purchase.submit
      order.mark_as_purchased!
    else
      # handle errors
    end
  end
end

order = Order.new
purchase = Purchase.new(order.amount, order.credit_card)
OrderController.new(order, purchase).run

Modules should:

should never have dependancies on other modules
should never ever EVER muck with the internal state of its containing object
should have as narrow an interface as possible with its containing object
should have a coherent reason for existing that makes sense in isolation
should be tested in isolation against a fake container

"Mixins are a form of inheritance, and have all the same pitfalls, so its usually better to err on the side of composition over mixins. I also think that the rails style of usage where you gut a massive class, and organize its methods by moving them into modules is terrible every time...To me, the ideal mixin is Enumerable. It is a way to inject advanced iteration behavior into any object that implements primitive iteration behaviors (the each method). It is focused, powerful, easy to implement, useful, and never gets tangled in the logic of its containing class."