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?
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 :)
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With