Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I am reading a book called Rails AntiPatterns and they talk about using delegation to to avoid breaking the Law of Demeter. Here is their prime example:

They believe that calling something like this in the controller is bad (and I agree)

@street = @invoice.customer.address.street

Their proposed solution is to do the following:

class Customer

    has_one :address
    belongs_to :invoice

    def street
        address.street
    end
end

class Invoice

    has_one :customer

    def customer_street
        customer.street
    end
end

@street = @invoice.customer_street

They are stating that since you only use one dot, you are not breaking the Law of Demeter here. I think this is incorrect, because you are still going through customer to go through address to get the invoice's street. I primarily got this idea from a blog post I read:

http://www.dan-manges.com/blog/37

In the blog post the prime example is

class Wallet
  attr_accessor :cash
end
class Customer
  has_one :wallet

  # attribute delegation
  def cash
    @wallet.cash
  end
end

class Paperboy
  def collect_money(customer, due_amount)
    if customer.cash < due_ammount
      raise InsufficientFundsError
    else
      customer.cash -= due_amount
      @collected_amount += due_amount
    end
  end
end

The blog post states that although there is only one dot customer.cash instead of customer.wallet.cash, this code still violates the Law of Demeter.

Now in the Paperboy collect_money method, we don't have two dots, we just have one in "customer.cash". Has this delegation solved our problem? Not at all. If we look at the behavior, a paperboy is still reaching directly into a customer's wallet to get cash out.

EDIT

I completely understand and agree that this is still a violation and I need to create a method in Wallet called withdraw that handles the payment for me and that I should call that method inside the Customer class. What I don't get is that according to this process, my first example still violates the Law of Demeter because Invoice is still reaching directly into Customer to get the street.

Can somebody help me clear the confusion. I have been searching for the past 2 days trying to let this topic sink in, but it is still confusing.

share|improve this question
2  
similar question here –  thorsten müller Oct 17 '13 at 9:02
    
I don't think the 2nd example (the paperboy) from the blog violates the Law of Demeter. It might be bad design (you're assuming the customer will pay with cash), but that's NOT a Law of Demeter violation. Not all design errors are caused by breaking this Law. The author is confused IMO. –  Andres F. Oct 17 '13 at 12:47
1  

4 Answers 4

up vote 17 down vote accepted

Your first example does not violate the law of Demeter. Yes, with the code as it stands, saying @invoice.customer_street does happen to get the same value that a hypothetical @invoice.customer.address.street would, but at each step of the traversal, the value returned is decided by the object being asked - it's not that "the paperboy reaches into the customer's wallet", it's that "the paperboy asks the customer for cash, and the customer happens to get the cash from their wallet".

When you say @invoice.customer.address.street, you're assuming knowledge of customer and address internals - this is the bad thing. When you say @invoice.customer_street, you are asking the invoice, "hey, I'd like the customer's street, you decide how you get it". The customer then says to its address, "hey I'd like your street, you decide how you get it".

The thrust of Demeter is not 'you cannot ever know values from objects far away in the graph from you"; it is instead 'you yourself are not to traverse far along the object graph in order to obtain values'.

I agree this may seem like a subtle distinction, but consider this: in Demeter-compliant code, how much code needs to change when the internal representation of an address changes? What about in non-Demeter-compliant code?

share|improve this answer
    
This is exactly the kind of explanation I was looking for! Thank you. –  user2158382 Oct 17 '13 at 9:16
    
Very good explanation. I have a questions: 1) If the invoice object wants to return a customer object to the client of invoice that doesn't necessarily mean that it's the same customer object that it holds internally. It may simply be an object created, on the fly, for the purpose of returning to the client a nice packaged data set with multiple values in it. Using the logic you present, you are saying that invoice cannot have a field that represents more than one data. Or am I missing something. –  zumalifeguard Oct 13 at 22:32

The first example and the second are actually not very same. While the first talking about general rules of "one dot", the second talks more about other things in OO design, especially "Tell, Don't ask"

Delegation is an effective technique to avoid Law of Demeter violations, but only for behavior, not for attributes. -- From the second example, Dan's blog

Again, "only for behavior, not for attributes"

If you ask for attributes, you are supposed to ask. "Hey, guy, how much money do you have in pocket? Show me, I'll evaluate if you can pay this." That's wrong, no shopping clerk will behave like this. Instead, they'll say, "Please pay"

customer.pay(due_amount)

It'll be customer's own duty to evaluate if he should pay and if he can pay. And the clerk's task is finished after telling customer to pay.

So, does the second example prove the first is wrong?

In my opinion. No, as long as:

1. You do it with with self-constraint.

While you can access all of the customer's attributes in @invoice by delegation, you rarely need that in normal cases.

Think about a page showing an invoice in a Rails app. There will be a section on top to show the customer's details. So, in invoice template, will you code like this?

#customer-info
  = @invoice.customer_name
  = @invoice.customer_address
  ....

That's wrong and inefficient. A better approach is

#customer-info
  = render partial: 'invoice_header_customer', 
           locals: {customer: @invoice.customer}

Then let the customer partial to process all attributes belongs to customer.

So generally you don't need that. But you may have a list page showing all recent invoices, there is a briefing field in each li displaying the customer's name. In this case, you do need the customer's attribute to show, and it's totally legit to code the template as

= @invoice.customer_name

2. There is no further action depending on this method call.

In above case of list page, the invoice asked the customer's name attribute, but it's real purpose is "show me your name", so it's basically still a behavior but not attribute. There is no further evaluation and action based on this attribute like, if your name is "Mike" I will like you and give you 30 days more credit. No, invoice just say "show me your name", no more. So that's totally acceptable according to the "Tell Don't Ask" rule in example 2.

share|improve this answer
    
Very detailed explanation. Thank you –  user2158382 Oct 17 '13 at 17:53

Read further in the second article and I think the idea will become clearer. The idea just have customer offer a capability to pay and completely hide where the case is kept. Is it a field, a member of a wallet, or something else? The caller doesn't know, doesn't need to know and does not change if that implementation detail changes.

class Wallet
  attr_accessor :cash
  def withdraw(amount)
     raise InsufficientFundsError if amount > cash
     cash -= amount
     amount
  end
end
class Customer
  has_one :wallet
  # behavior delegation
  def pay(amount)
    @wallet.withdraw(amount)
  end
end
class Paperboy
  def collect_money(customer, due_amount)
    @collected_amount += customer.pay(due_amount)
  end
end

So I think your second reference is giving a more helpful recommendation.

The "one dot" only idea, is a partial success, in that it hides some deep detail, but still inbcreases coupling between separate components.

share|improve this answer
    
Sorry maybe I wasnt clear, but I understand the second example perfectly and I understand that you need to make the abstraction you posted, but what I dont understand is my first example. According to the blog post, my first example is incorrect –  user2158382 Oct 17 '13 at 9:01

Sounds like Dan derrived his example from this article: The Paperboy, The Wallet, and The Law Of Demeter

Law Of Demeter A method of an object should invoke only the methods of the following kinds of objects:

  1. itself
  2. its parameters
  3. any objects it creates/instantiates
  4. its direct component objects

When and How to Apply The Law Of Demeter

So now you have a good understanding of the law and it's benefits, but we haven't yet discussed how to identify places in existing code where we can apply it (and just as important, where NOT to apply it...)

  1. Chained 'get' Statements - The first, most obvious place to apply the Law Of Demeter is places of code that have repeated get() statements,

    value = object.getX().getY().getTheValue();

    as if when our canonical person for this example were pulled over by the cop, we might see:

    license = person.getWallet().getDriversLicense();

  2. lots of 'temporary' objects - The above license example would be no better if the code looked like,

    Wallet tempWallet = person.getWallet(); license = tempWallet.getDriversLicense();

    it is equivalent, but harder to detect.

  3. Importing Many Classes - On the Java project I work on, we have a rule that we only import classes we actually use; you never see something like

    import java.awt.*;

    in our source code. With this rule in place, it is not uncommon to see a dozen or so import statements all coming from the same package. If this is happening in your code, it could be a good place to look for obscured examples of violations. If you need to import it, you are coupled to it. If it changes, you may have to as well. By explicitly importing the classes, you will begin to see how coupled your classes really are.

I understand that your example is in Ruby, but this should apply across all OOP languages.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.