Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Law of Demeter for Views: create delegates to access attributes on associated objects or not?

The Law of Demeter seems to be a very powerful concept. I can understand how it helps writing good and maintainable object-oriented code.

Some people suggest to write a delegate method each time you need to access an attribute of an associated object in a view. Instead of writing something like this in a view

@order.customer.name

you would write this code:

# model
class Order < ActiveRecord::Base
  belongs_to :customer
  delegate :name, :to => :customer, :prefix => true
end

#view
@order.customer_name

On the other hand, people argue that you views should not dictate models and you should not add methods such as delegate to a model only for the sake of trading a dot for an underscore in a view.

When violating the Law of Demeter in a view, is it considered best practice to write delegate methods in models or not?

like image 491
migu Avatar asked Jun 01 '13 03:06

migu


1 Answers

I see your customer_name auto-generated delegate method as the Simpliest Thing That Works Right now. Since it's one method call (and not a series of method chains) it's easy to refactor later (or, easier to refactor than some chained methods)

Imagine adding many customers to an order, one of which is the primary customer, for whatever reason. Now your order class might look like

class Order < ActiveRecord::Base

  has_many :customers

  def customer_name
    if customers.first.primary?
      customers.first.name
    else
      customers.last.name
    end
end

It was easy to replace that convenience delegate generated method with one of our own.

(It's also super easy to write the first time, as delegate takes care of all the boilerplate. It's very possible you'll use customer_name in this form forever in your app. It's hard to know. But code that's easy/automatic to write the first time is cheap to throw away :))

Of course you have to avoid situations where you are writing method names like customer_streetaddress_is_united_states? (where yes, instead of encoding the object graph in dots you're encoding it in underscores.)

If your view really needs to know if the user is located in the US perhaps a method like this might work:

class Order < ActiveRecord::Base

    belongs_to :customer

    def shipping_to_us?
      customer.shipping_country == "USA"
      # Law of Demeter violation would be:
      # customer.addresses.first.country == "USA"
    end
  end

  class Customer < ActiveRecord::Base
    has_many :addresses

    def shipping_country
      addresses.first.country
    end
  end

Notice here how the Order asks the Customer object for the shipping address, vs telling the customer to get it's customer's first address's country. Like a boss that tells you to do something and leaves you alone vs a boss that micromanages exactly how you do your day to day. (For additional edification, read up on the ask, don't tell approach to Ruby development :) )

There is something to be said about using presenters, decorator methods, or helpers to avoid having this potentially just display logic code littering your models. I'll leave that as an exercise for the reader :)

like image 65
RyanWilcox Avatar answered Dec 01 '22 01:12

RyanWilcox