Demeter: It’s not just a good idea. It’s the law.

Is #try really so bad?

In response to my recent post about #try being a code smell, a lot of people made the reasonable objection that the example I used—of using #try on a a Hash—was a pathological case. A much more typical usage of #try looks like this:

def user_info(user)
  "Name: #{user.name}. Dept: #{user.department.try(:name)}"
end

user may or may not have an associated department, so the call to Department#name is wrapped in a #try. If there is an associated department, its name will be returned. If not, the result will be nil.

Straightforward enough. Is there anything wrong with this code?

I can think of a a few things. For one thing, it’s ugly. For my money, one of the hallmarks of beautiful code is that it’s visually consistent: similar operations have similar appearance. In the code above, we access one attribute with simple dot syntax (.name), and another with a very different-looking .try(:name), even though in both cases the concept we are trying to express is the same: “get the ‘name’ attribute”.

It’s a variety of ugliness that tends to proliferate, too. Starting with the code above, it’s not a very big leap to get to this:

def user_info(user)
  "Name: #{user.name}. Boss: #{user.department.try(:head).try(:name)}"
end

Yuck.

And then there are the tests. They’ll probably look something like this:

describe '#user_info' do
  subject { user_info(user) }
  let(:user) { stub('user', :name => "Bob", :department => stub(:name => "Accounting")) }
  specify {
    subject.should match(/Dept: Accounting/)
  }
end

Metastasizing mocks

This, again, doesn’t seem so bad. But give that test suite six months of active development, and chances are the tests wind up looking more like this:

describe '#user_info' do
  subject { user_info(user) }
  let(:user) { 
    stub('user', 
         :name => "Bob", 
         :department => 
           stub(:name     => "Accounting",
                :head     => 
                  stub(:name     => "Jack", 
                       :position => stub(:title => "Vice President"))),
            :division => stub(:name => "Microwave Oven Programming")),
          :position => stub(:title => "Senior Bean Counter"))
 }
  # examples...
end

Not only that, the same tree of stubs will probably be duplicated, with subtle differences, for every test group that interacts with a User—because no one has time to sort out the specific subset of stubs that a given test actually needs in order to function.

At some point the client will decide that users really need to be associated with zero or more departments instead of just one. At that point some unlucky programmer will spend a late night fixing the 300 tests this “small change” breaks because of all the stubs that model the old behavior. Then the next day he’ll write an angry rant about how mock objects are a bad idea.

Structural coupling

The seed of this all-too-common predicament is structural coupling. What’s structural coupling? To define it, let’s start with a review of the DRY principle:

Every piece of knowledge must have a single, unambiguous, authoritative representation within the system.

It’s easy to think about DRYness just in terms of data: e.g., there should be only one place in the system for API keys; they shouldn’t just be copy-and-pasted willy-nilly throughout the codebase. But DRY applies equally to structural knowledge: knowledge about the composition of and relationships between your objects.

Let’s take a look at the code we started out with:

def user_info(user)
  "Name: #{user.name}. Dept: #{user.department.try(:name)}"
end

This seemingly innocuous code makes the following assumptions:

  • user will have a name property.
  • user may or may not have a single department.
  • user‘s department, in turn, has a name property

By going two levels deep into user‘s associations, we’ve made a structural coupling between this code and the models it works with. We’ve duplicated knowledge about a User’s associations—canonically located in the User and Department classes—in the #user_info method.

And the #try method was an enabler. By papering over the ugly user.department && user.department.name construct we’d otherwise have had to use, #try made the coupling an easier syntactical pill to swallow.

This would be bad enough if we made a habit of it, because we’d have to change every method with a similar structural coupling whenever the innards of User or Department changed. But because we’re good Test-Driven developers, we then proceeded to couple dozens of test suites to a specific model structure, in the form of stubs and mock objects.

This is clearly an undesirable outcome. Wouldn’t it be handy to have a simple rule that helps us avoid structural coupling?

The Law of Demeter

Back in the 1980s, a group of programmers working on a project called the Demeter system realized that certain qualities in their object-oriented code led to the code being easier to maintain and change. Qualities such as low coupling; information hiding; localization of information, and narrow interfaces between objects. They asked themselves: “Is there a simple heuristic that humans or machines can apply to code to determine whether it has these positive qualities?”.

The answer they came up with came to be known as the “Law of Demeter”. It is stated as follows:

For all classes C. and for all methods M attached to C, all objects to which M sends a message must be instances of classes associated with the following classes:

  1. The argument classes of M (including C).
  2. The instance variable classes of C.

(Objects created by M, or by functions or methods which M calls, and objects in global variables are considered as arguments of M.)

WikiWiki explains the law like this:

  • Your method can call other methods in its class directly.
  • Your method can call methods on its own fields directly (but not on the fields’ fields).
  • When your method takes parameters, your method can call methods on those parameters directly.
  • When your method creates local objects, that method can call methods on the local objects.

If that still seems confusing, here’s an alternative explanation from Peter Van Rooijen:

  • You can play with yourself.
  • You can play with your own toys (but you can’t take them apart),
  • You can play with toys that were given to you.
  • And you can play with toys you’ve made yourself.

The Demeter programmers wrote up their experiences in a paper called Object-Oriented Programming: An Objective Sense of Style. What they found was that when methods were written in a form which complied with the Law of Demeter, the resulting codebase was easier to maintain and evolve.

It’s important to understand that the Law of Demeter is a heuristic, not an end in and of itself. It is not a law in the sense that you “must” write your code in a certain way. Rather, it is a law in the sense that it has been consistently observed that if code complies with the Law of Demeter, it almost certainly has a number of the qualities—encapsulation, loose coupling, etc.—desirable in an OO system.

Laying down the law

With that in mind, let’s take one more look at our example code:

def user_info(user)
  "Name: #{user.name}. Dept: #{user.department.try(:name)}"
end

This code does not comply with the Law of Demeter. In addition to calling methods on its parameter, user, it also calls a method on the result of one of those methods: (department.name).

Assuming this is a Rails program, it is extremely easy to change the code to satisfy the law. First, we make a one-line addition to the User class:

class User
  delegate :name, :to => :department, :prefix => true, :allow_nil => true
  # ...
end

The #delegate macro, provided by ActiveSupport, generates a new method User#department_name which delegates to the user’s #department. By supplying :allow_nil => true, we ensure that the method will simply return nil in the case when there is no department associated with the user.

Here’s our code again, updated to use the new method:

def user_info(user)
  "Name: #{user.name}. Dept: #{user.department_name}"
end

The code now respects the Law of Demeter: it is coupled only to the immediate interface of the user parameter.

The updated test suite now has only one stub object:

describe '#user_info' do
  subject { user_info(user) }
  let(:user) { stub('user', :name => "Bob", :department_name => "Accounting") }
  specify {
    subject.should match(/Dept: Accounting/)
  }
end

Already we have a simpler test suite. But the real benefit comes when it is time to change the models. Let’s consider the case when a User changes from being linked to just one department, to having a list of zero or more departments. How we re-implement User#department_name depends on the needs of the domain. Let’s say the department name should now be a comma-separated list:

class User
  def department_name
    departments.join(", ")
  end
end

We replace our delegate method with a method implementing the new semantics. And that’s the only change! The #user_info method remains the same, as does every test suite that references User#departmentname.

By adhering to the Law of Demeter, we have decreased coupling, and increased the velocity with which we can make changes to the business logic.

Objection #1: What about method chains?

“But Avdi” you may object, “it sounds like a good guideline, but clearly it’s not something to be rigidly adhered to in Ruby code. If we followed it all the time we could never do method chaining!”

Method chains are a core Ruby idiom, to be sure. As an example, here’s a method which takes a string and generates a “slug” for use as an identifier or as a a URL component:

def slug(string)
  string.strip.downcase.tr_s('^[a-z0-9]', '-')
end

That’s one, two, three levels of method call. That can’t comply with the Law of Demeter, but it surely is concise and convenient!

Look again at the definition of the Law: it never says anything about the number of methods called, or the number of objects a method uses. It is strictly concerned with the number of types a method deals with.

The #slug method expects a String, and calls three methods, each one returning… another String. In fact, because it only calls methods for the type of object (String) passed into it as parameters, we find that this method complies perfectly with the Law of Demeter.

Likewise with another common Ruby pattern, chains of Enumerable methods like #map and #select. Because each returns another Enumerable object, there is no violation.

Objection #2: Delegation explosion

Another objection to Demeter is that strictly following it results in objects which are full of attributes which aren’t a direct part of their responsibility. Quoting Mark Wilden in the comments on my previous article:

Why should a Human have to know whether a Country has a name? Or any other attribute (unless it needs them itself)? If a Human is associated with multiple Countries (birth, residence, voting, vacation, etc.) does it then have to duplicate this delegation for each method of each country?

What about attributes that clearly have nothing to do with Human? Yes, one might say that a Human has a country_name. But does a Human have a country_population? A country_mortality_rate? I would say it does not, but Demeter insists that it must.

In the Object-Oriented view of the world, objects are not merely bags of attributes. They are entities to which you send messages and from which you receive replies. The classic example is a financial transaction: if I am a shopkeeper and you buy something from me, I don’t ask you for your wallet, rummage around until I find a credit card, and then copy down the information I need. Instead, I ask you for your credit card number and expiration date.

Putting this in object terms, a payment system which calls person.wallet.credit_cards.first.number exhibits tight structural coupling, and is closer in spirit to the data-structure-oriented programming which preceded OO. From an objects-sending-messages standpoint, it is perfectly reasonable for a Person to have a creditcard_number.

An important and often neglected point to hit on, before we move on: in an Object-Oriented system, it is prfectly allowable (and even encouraged) for objects to have personas or facets. A doctor deals with a patient’s physical symptoms while the reception desk deals with her wallet and insurance info. You wouldn’t walk into a doctor’s office, step up to the receptionist, and take off your shirt (unless you were on very good terms with the receptionist!).

Likewise, an object can have a large API, but only expose subsets of that API to different collaborators. Some languages enforce these subset relationships quite strictly; e.g. C++ with its private inheritance, and interfaces in Java. In other languages, such as Ruby, the restriction may be more about convention than something the language enforces. There’s nothing wrong with having a large API, so long as individual collaborators only talk to well-defined subsets of it.

But what about Mark’s example human.country_mortality_rate? Surely that’s pushing it a bit far?

Perhaps it is. But Demeter doesn’t prevent us from interacting with an objects second- and third-order associations; it simply asserts that we can’t interact with all of those objects in the same method. Look again at the formulation of the law:

…all objects to which M sends a message…

Demeter is a rule about methods only; it does not limit the set of types a class can interact with.

So this is perfectly legal:

class StatPresenter
  def human_stats(human)
    "Age: #{human.age}.nCountry stats:n#{country_stats(human.country)}"
  end

  def country_stats(country)
    "  Mortality rate: #{country.mortality_rate}"
  end
end

Of course, you could completely violate the spirit of Demeter by taking this too far; something the authors of the Demeter paper note. Realistically, we’d probably want to break that StatPresenter class up into smaller classes once it started interacting with many different types of object.

The important thing, from the standpoint of Demeter, is to avoid tying a single method to a deep hierarchy of types, as well as limiting the number of types one method deals with.

One of the most basic ways we can limit the number of types a given method must be aware of is to eliminate the common case of “maybe nil” parameters. Remember, NilClass is a type too, and when a parameter might be nil we’ve increased the number of types the method has to know about by one.

As an example, the following version of the code above, while technically Demeter-compliant, is once again riddled with #try calls:

class StatPresenter
  def human_stats(human)
    "Age: #{human.age}.nCountry stats:n#{country_stats(human.country)}"
  end

  def country_stats(country) # country may be nil
    "  Population: #{country.try(:population)}n" +
    "  Mortality rate: #{country.try(:mortality_rate)}n"
  end
end

The set of types #country_stats deals directly with is: StatPresenter (self), Country, and NilClass.

We can’t always get rid of switching on nil entirely, but what Demeter-influenced code gives us the opportunity to do is to easily confine that switch to a single location. Let’s rewrite the code above:

class StatPresenter
  def human_stats(human)
    "Age: #{human.age}." + (human.country ? 
      "nCountry stats:n#{country_stats(human.country)}" :
      "n(No Country Stats)")
  end

  def country_stats(country)
    "  Population: #{country.population}n" +
    "  Mortality rate: #{country.mortality_rate)n"
  end
end

With this final edit, we’ve reduced the coupling of each method to a minimal point. StatPresenter#human_stats deals only with Human objects, and all it knows about #country is that it may or may not be there. StatPresenter#human_stats only knows about Country objects.

Bringing Demeter to work

“OK, fine. I can see that the Law of Demeter is a great guideline, at least in theory. But who has time to do all that refactoring? I have deadlines to meet!”

While refactoring code to comply with Demeter can certainly improve its design, I don’t think Demeter becomes truly practical until you incorporate it consistently into your coding style. Like many low-level “code construction” techniques—such as good variable naming—its value lies less in coming in and applying it after the fact, and more in practicing it until it becomes second nature.

Let’s take a look at how we’d add the department name to the #user_info method using TDD and the Law of Demeter. Here’s the code before adding the new functionality:

def user_info(user)
  "Name: #{user.name}"
end

Now let’s add the department name.

  1. We write our test first:

    describe '#user_info' do
      subject { user_info(user) }
      let(:user) { stub('user', :name => "Bob", :department_name => "Accounting") }
      specify {
        subject.should match(/Dept: Accounting/)
      }
    end
    

    We know that nested mock/stub objects is a smell indicating structural coupling, so we force ourselves to write a stub for the user object we wish we had. The User#department_name method doesn’t exist yet; we make a mental note to implement it. If we forget, the omission will be caught by our acceptance and/or integration tests.

  2. We run the spec. It fails, because we haven’t implemented it yet
  3. We write enough code to make the test pass:

    def user_info(user)
      "Name: #{user.name}. Dept: #{user.department_name}"
    end
    
  4. We run the tests again, and this time they pass.
  5. The final step is to implement the User#department_name method. We could write a test asserting that the method delegates to Department; personally, I find this a little redundant and would just write the delegation and call it done:

    class User
      delegate :name, :to => :department, :prefix => true, :allow_nil => true
      # ...
    end
    

Revisiting your code to make it Demeter-compliant after the fact will indeed slow you down. By incorporating the rule into your habits, to the point that it becomes second nature, you reduce the impact (if any) to the point where it becomes insignificant. This is especially true in Ruby and Rails, where techniques such as composition-and-delegation, viewed as “heavyweight patterns” in some languages, become one-liners. And any fractional slowdown you do experience from an extra test run here and there will be more than made up for by the ease of changing your loosely-coupled code as requirements change. With discipline and practice, it is possible to be both fast and good.

Conclusion

To summarize:

  • #try is more often than not indicative of structural coupling. Structural coupling, in turn, violates the DRY principle.
  • Structural coupling, left unchecked, can substantially slow the evolution of a project.
  • The Law of Demeter, which sets limits on the number of types a single method can interact with, is a heuristic for identifying code that (among other positive properties) has low structural coupling.
  • When refactor our code to comply with the Law of Demeter, it tends to reduce structural coupling both in application code and in tests. As a side effect, it tends to eliminate the need for #try calls and similar constructs.
  • Contrary to popular belief, Demeter does not limit the number of of dots in a method call chain. It also doesn’t limit the number of classes a class can interact with.
  • The best way to incorporate Demeter into your work is to make it a habit, rather than a cleanup chore.

Do you look for Demeter violations in your code? Do you think there are still some instances where a #try makes sense? Do you have more questions about the Law of Demeter or structural coupling? As always, I welcome feedback in the comments!

P.S. It’s my birthday! To celebrate, for 24 hours I’m offering 50% off on my book, Exceptional Ruby. Use code HAPPY0X1F to get the discount.

This entry was posted in Ruby and tagged , , , , , , , , , , , , . Bookmark the permalink.
  • http://twitter.com/fxn Xavier Noria

    But again, you’re solving a different problem.

    You say try is wrong because of demeter. No, what’s wrong is the interface. The programmer is violating demeter, not try. If AS didn’t have delegate you could implement department_name using try just fine. Even if AS provides delegate, you could, less declarative, but you could and it would be an ordinary use case of try.

    • http://avdi.org Avdi Grimm

      You’re exactly right.

      Except about what I said. I said #try is a code smell, not that it was wrong.

      The definition of a code smell:

      code smell is any symptom in the source code of a program that possibly indicates a deeper problem

      In nearly every use of try I have ever seen, it indicates a design flaw with deeper ramifications: either a failure to use polymorphism, or a failure to respect Demeter (that is, an introduction of structural coupling). Addressing the design issue almost invariably obviates the use of #try. You are correct: #try is a sign of the problem, not the problem itself. 

      That is why I am confident in saying that #try is a code smell.

      • http://twitter.com/fxn Xavier Noria

        I disagree Avdi.

        In that use case try is nothing special. You could write the same blog post about the expression user.department.name. So albeit the blog post is obviously about the law of demeter, its relationship with the example code using try is only casual.

        In that particular case, in addition, using try is fine for me. If the user has a department in its interface, then it is OK. And that is a natural interface to provide. As in book.author. It doesn’t scale to six levels, yes, but for n = 1 it looks fine to me.

        You know I don’t think try is a code smell. Being a code smell is a statistical property, so we may have different experiences regarding this. If I had to write an automated tool to report smells, I wouldn’t look for usages of try.

        • Aaron Patterson

          Using `try` increases the number of types your method is OK to deal with.  You’re increasing the number of code paths through your method.  Where you had one code path to test, now you have two.  I don’t think the relationship with `try` is casual at all, it’s a common way that people increase the number of types they deal with in their methods.

          For me, increasing the number of types / code paths is definitely a code smell.  Places where `try` is used are fine examples of complexity increasing unnecessarily.

          • http://twitter.com/fxn Xavier Noria

            Well, department.nil? ? nil : department.name, how many code paths do you have there? Regarding types, I don’t thing that matters really, formally there’s a type, but it is one that belongs to the output you want. You use try in situations where you either get nil or a String.

  • Christopher Johnston

    This is probably one of the best articles on the Law of Demeter and the ways, in Ruby, to go about solving the problem. I really like the way in which is agrees with Domain Driven Design and the idea of aggregate types exposing an interface to nested objects within the graph.

  • Christopher Johnston

    This is probably one of the best articles on the Law of Demeter and the ways, in Ruby, to go about solving the problem. I really like the way in which is agrees with Domain Driven Design and the idea of aggregate types exposing an interface to nested objects within the graph.

    • http://avdi.org Avdi Grimm

      Glad you enjoyed it!

  • http://iain.nl iain

    Happy birthday!

    On a code retreat I  did last week I found something similar: If you write your unit tests really isolated, and you obsessively stub/mock out the collaborators, than a violation of the law of demeter becomes really easy to spot: in the unit tests you’ll have nested stubs.

    So I’ll see this article as a confirmation of my observation and it’s articulated better than I ever could. Thanks!

    • http://avdi.org Avdi Grimm

      Yeah, it’s a great example of how some practices really only work well when used in conjunction. Mocks are great–IF you have the discipline to listen to them and factor your code accordingly.

      • http://mwilden.blogspot.com Mark Wilden

        Just as another data point, I find that using factories removes this awkwardness. However, that’s a different subject.

        • http://iain.nl iain

          Yes, using factories, or real objects, removes the awkwardness of (nested) stubs. But you’ll lose the heuristic to find the Demeter violation. The repetition of the word ‘stub’ is a warning you shouldn’t ignore. I usually say that with tests you need to “feel the pain” so to speak. And if you take a sedative in the form of a tool that hides this pain from you, you’re wise to know *exactly* what problem you are hiding.

          • http://avdi.org Avdi Grimm

            Yes! Exactly.

          • http://mwilden.blogspot.com Mark Wilden

            Hmmm…this seems a little circular to me. Demeter says a method shouldn’t drill down into subobjects. If you do that, then it’s harder to stub. I’m saying (basically) that a method should drill down, and that it’s easier to use factories. The two arguments are orthogonal – neither proving nor disproving the other. I have to admit, however, that since I started using factories, I use stubs much less. Making the top-level object a gatekeeper for its subobjects does make stubbing easier, but then so would having one uber-object that knew about every other object in the application (to reduct a little absurdity).

          • http://iain.nl iain

            I’m not disagreeing that factories make it easier. Factories are incredibly useful and I wouldn’t want to do integration testing without it. But using factories because making nested stubs is too difficult is the wrong reason to use factories.

            Factories are in a different problem domain than the Law of Demeter. Demeter is concerned about writing maintainable code. Factories don’t help you write more maintainable code, only more maintainable test suites.

            Both Demeter and “Stub awkwardness” are ways to tell if your code can be simpler. You shouldn’t write nested stubs, you should see that you need nested stubs and refactor your code so that you shouldn’t have to.

            I use stubs mostly when test driving out a design. It is a place holder for something I don’t want to think about yet. When you start out with stubs, you’ll never write a deeply structured object, because that would be too painful.

          • Pete Yandell

            I completely agree, Iain. A lot of the support requests I get for Machinist are people using Machinist as a crutch for bad design, and that design getting so bad that Machinist can’t hold up the tests anymore.

            It usually looks like this:

            An ActiveRecord model has been in there code base a while, and has grown pretty big. It now has a bunch of methods that drill down through several layers of associations. Now, whenever you use the model in a test, you need to create an instance of the model, its associated objects, their associated objects, etc.

            In Machinist 2, I’ve taken out a lot of the functionality around associations, in the hope that it’ll discourage this sort of thing.

            I’m contemplating a blog post on how you start to clean up a code base that has got into this state.

          • http://stefanoverna.com Stefano Verna

            > An ActiveRecord model has been in there code base a while, and has grown pretty big. It now has a bunch of methods that drill down through several layers of associations. Now, whenever you use the model in a test, you need to create an instance of the model, its associated objects, their associated objects, etc.

            Pete, this is exactly the problem I’m facing these days. Please, write that post for me..
            or, at least, share with me a couple of smart tips :) Thanks!

        • http://avdi.org Avdi Grimm

          I thought of putting that in the article, but decided it was a distraction. Once you switch to factories you have two new problems: you aren’t unit testing anymore, and your unit tests are slow as molasses.

  • http://iain.nl iain

    Happy birthday!

    On a code retreat I  did last week I found something similar: If you write your unit tests really isolated, and you obsessively stub/mock out the collaborators, than a violation of the law of demeter becomes really easy to spot: in the unit tests you’ll have nested stubs.

    So I’ll see this article as a confirmation of my observation and it’s articulated better than I ever could. Thanks!

  • Guest

    I’m really not comfortable with the geometric growth in an object’s interface that this would lead to (and the subsequent cascade to its callers?). Instead, I’d prefer to introduce a pair of additional types just to mediate the relation between humans and countries, and humans and departments. These new types then become the only way of referring to humans and departments/countries at once, satisfying both DRY and the SRP. Disclaimer: I’m not a Rubyist, and come from a background that loves its types so I might be ignoring something obvious.

    • http://avdi.org Avdi Grimm

      Longtime readers of my blog know that I’m also a big proponent of decorators, which I find are frequently a good way of encapsulating roles which may not make sense for an object to play all of the time.

      • http://railscasts.com Ryan Bates

        Do you think that there is a balance here? In practice I find applying the Law of Demeter strictly and aggressively can turn even the simplest programs into a mess of indirection and delegation. In your example you showed how the code can get worse if this is ignored, but when it starts to get worse (department.head.name) then that is when it’s time to apply it IMO.

        Over-refactoring can be more dangerous than under-refactoring.

        I would love to see an experiment where a full application/problem is written strictly following the Law of Demeter compared to where it is written in a more direct approach.

        • http://avdi.org Avdi Grimm

          Here’s the problem: refactoring is more tedious and less fun and developers always put it off. It’s human nature. Plus Ruby doesn’t have the greatest refactoring tools, and a lot of Ruby devs aren’t even using the ones that do exist.

          In my work I get to see lots of legacy Rails projects that have built up over the years. And what I’ve found is that if you don’t make this small-scale stuff a habit, it doesn’t get done.

          There are some refactorings that you absolutely have to do after the fact, because otherwise you’ may be prematurely overdesigning.  Other design issues, however, are so small, and become so pervasive if left till later, that it makes sense to simply make a habit of not committing them in the first place. At the level of Code Constructions some things make sense to just get in the habit of doing them right every time. 

          I wouldn’t ever name a variable “foo” because I can’t be bothered to take the time to give it a meaningful name. Likewise, structural coupling is something that is easier to simply avoid using a few simple rules than it is to constantly be coming back and cleaning up.

          • http://railscasts.com Ryan Bates

            I agree some design decisions are better made up-front, but I was mainly wanting clarification about your view of the Law of Demeter. Do you apply it strictly in every situation or do you find there’s a balance and tipping point on when it becomes necessary?

            Also, do you know of any open source projects you or others have done which I can take a look at that follow this pattern strictly?

          • http://avdi.org Avdi Grimm

            I try to apply it with discipline. That said, you could probably find plenty of examples of hypocrisy in my own projects. It’s an ongoing process.

            Good question about projects. To me, the most interesting projects to apply this stuff to are commercial applications, because that’s where the rubber  meets the road as far as practices paying off (or not) in real dollar value. It’s also where I spend 95% of my time. But those are also the projects that are unavailable for public viewing.

            You can look at OSS projects too, but there’s a lot more leeway there for people to refactor because they feel like it, divorced from any schedules, so I don’t feel like I can point to an OSS project and say “see? here’s where clean code paid off!”. Maybe that project has “clean code” just because someone was bored and anal retentive. Conversely, maybe it has horrible code but it doesn’t matter because no one’s demanding that it make major course changes on a deadline.

  • http://twitter.com/SusanPotter Susan Potter

    To help underline one of the most important (IMO) points you make in your blog post I wrote a Gist with comments here: https://gist.github.com/1066237

    I felt the use of the delegate class method from ActiveSupport in your proposed solution is detracting from the principle a little, which is why I kept it simple and not assumed AS or ActiveRecord classes.

    I think the key is the interfaces used between objects (depending on their relationship to one and other) should be well defined. When this occurs it is almost certain that the Law of Demeter will be complied with as a side effect.

    • http://avdi.org Avdi Grimm

      I take your point about uniform interfaces, but I’ll challenge you with this: I kind of glossed over this in the article, but most domain objects have no business having a to_info, a to_json, to_xml, or any similar method. Those are presentation concerns, and logically belong in some kind of Presenter object, which is where I typically put them in any program bigger than a toy. 

      • http://mwilden.blogspot.com Mark Wilden

        Now that I completely agree with! :) I made much the same comment in my blog article about why I don’t like pair programming – my pair insisted on having domain objects know how to graph themselves, which led to foolishness such as knowing which one was selected in the UI.

      • http://twitter.com/SusanPotter Susan Potter

        I wasn’t advocating having putting presentation inside a model, but that is the *purpose* of your #user_info method in the first place. That wasn’t my idea:) I suggest you consider revising your example as it detracts from the real issue at stake.

        • http://avdi.org Avdi Grimm

          Something I’ve discovered in years of writing these things is that no matter how many times I re-write–even when I have others review the article before publishing, as I did with this one–there is always something I wind up wishing I could revise. At some point I just have to call it done and move on to the next topic.

          This particular article suffers from what I will call the “hello world” problem: nobody actually needs a “hello, world” program, but the world is littered with them because they are a convenient way to say “ignore what this program is doing, and focus on the how“. In a fully mature program I probably wouldn’t put presentation logic in models like this; but generating strings is a trivial application that anyone can understand without extra explanation, so I used it for illustration.

          If this intentional simplification obscures the intent, I apologize.

      • Steve Wright

        You did introduce the user_info method for what appears to be a representational purpose rather than a domain logic purpose in the article itself so sermonizing about it to a commenter that makes a valid point of using well defined interfaces to solve the problem instead of Ruby “magic” (as you did) is undeserved.

        I too found the interfaces paragraph the real point of the Law of Demeter and really where the emphasis should have been placed in the article. You obviously know the right things, but instead you opt to use delegate (more on this later).

        To try to offer more constructive criticism to the overall article I would like to suggest the following to improve the article or to provide some counter points to those made in the article:
        * The example you gave was not “real” enough. It returned a string representation of the object with its internal values. (Violating the presentation not part of domain objects rule.)

        * Your solution is to use an optional library API (rather than a Ruby stdlib/core facility) that simply offers syntactic sugar. I searched for a link to include here on Ruby forwardables and oddly found (on the 2nd or 3rd search results page) an article from a Susan Potter (the same as the commenter here perhaps?) on the topic. Interestingly this blog post from 5 years ago says, “This also helps us implicitly adhere to the Law of Demeter (when not misused)”. See [1] for this article and [2] and [3] for other forwardables documentation (it is part of the core libraries). The danger you run into by providing a non OO-solution (it relies on Ruby “magic”/syntactic sugar, not general OO principles) is that you do not really demonstrate how a realistic modeling problem (first, of course, you need a non-trivial example) can be solved using good old fashioned OO design (and it is possible).

        * I could easily argue that your use of delegate in your “solution” is very much a code smell itself (as could be using forwardables). The purpose of the Law of Demeter isn’t to mandate “follow this rule”, it is to say when you design a good, maintainable OO layer that represents your domain well, this rule seems to hold. Therefore, it could offer insight to how well designed an OO codebase is, but it shouldn’t be thought of in the flip way as it appears you are thinking about it.

        * I do not see #try any more of a code smell than delegate (or even forwardable’s construct) itself. The evidence you provide is not really there. How does it differ? It wasn’t explained well enough (IMHO) for me to tell if you have a bigger point that I missed.

        [1] http://geek.susanpotter.net/2006/07/rubyisms-forwardables.html
        [2] http://www.ruby-doc.org/stdlib/libdoc/forwardable/rdoc/index.html
        [3] http://railsmagazine.com/articles/4

        • http://avdi.org Avdi Grimm

          Rather than re-paste, I will refer you to my reply to Susan above: http://avdi.org/devblog/2011/07/05/demeter-its-not-just-a-good-idea-its-the-law/#comment-247495349

  • Mark Lee Smith

    I haven’t read the paper you linked yet (thanks for that) however it seems to me that a more beautiful way to go from a department to departments would be to encapsulate departmental collaboration as an object with the same interface as a department, but which answers appropriate composite names, etc.

    The idea of operating on groups as a though they were singular entities is not new and has a number of very nice practical and cognitive advantages.

    Moreover In this example the implementation of User does not need to change at all; users belonging to a single department do not need to manage an array of departments that requires department_name and other methods to be reimplemented. Rather you just provide the appropriate departmental object and everything just works as it did before.

    (And if you serialised or otherwise wrote objects to disk, a database etc. you can still read those files.)

    If I’m missing something please let me know :).

    Good article. Got me thinking.

    • http://avdi.org Avdi Grimm

      I think that’s a completely reasonable approach. Thanks for the comment!

    • http://avdi.org Avdi Grimm

      I think that’s a completely reasonable approach. Thanks for the comment!

  • http://railscasts.com Ryan Bates

    Thanks for the insight into the law of Demeter, Avdi. It’s great to look at it as types rather than method chains.

    On a somewhat unrelated note, this seems to encourage testing through stubs. I find stubs to be a slippery slope because as soon as you use them in one location they seem to spread until every unit test is using stubs. This leads to brittle tests which cause simple refactorings to break many tests even though the method produces the same result. For this reason I’ve shied away from using stubs except when needing to mimic some external API. I would love to hear your thoughts on this. Perhaps in another blog post?

    • http://avdi.org Avdi Grimm

      I feel like I already covered it in this post, but maybe I could expand on it in another one. The point I tried to make above is that your stubs and mocks will metastasize unless you listen to them and maintain good design discipline at every step. Mocks work great–they are essential to true unit testing–but just like TDD only works if you are disciplined about red-green-refactor, mocks only work if you use them in conjunction with rules like Demeter as well as a either an acceptance or an integration suite which tests the integrations between units.

      • http://railscasts.com Ryan Bates

        Out of curiosity, do you stub out everything you unit test? That is, never passing the real objects around, only stubs. If not, how do you decide when to stub and when to use the real object?

        • http://avdi.org Avdi Grimm

          Yes, by definition. A Unit Test is a test that tests a single program unit
          (object or method) in isolation.

          When I find I have a reason to test Units together but it’s too low-level to
          warrant a new acceptance test (often the case when I need to verify
          variations on complex database interactions, for instance), I move the
          affected tests, either to a “functional/” or “integration/” directory; or at
          the very least to a separate some_model_functional_spec.rb file. This extra
          work keeps me honest, reminding me to only load down the tests that
          absolutely require it with extra dependencies, and keep the Unit tests
          isolated (and fast!).

          • http://mwilden.blogspot.com Mark Wilden

            Just a nitpick: I think it’s more accurate to say that a unit test tests a single non-core/non-library unit. Otherwise, you’d have to mock out operations on numbers, strings and hashes.

          • http://avdi.org Avdi Grimm

            Fair point :-)

      • http://railscasts.com Ryan Bates

        Out of curiosity, do you stub out everything you unit test? That is, never passing the real objects around, only stubs. If not, how do you decide when to stub and when to use the real object?

    • http://avdi.org Avdi Grimm

      Continued thoughts… I’ve worked on lots of projects that had this brittle mock problem. In every case, without fail, it indicated design shortcomings. Not large-scale design, but low-level design of the kind I’m talking about in the article. As a general rule, if you have more than a couple of simple mocks for a test suite, your suite is telling you something about how your objects are factored (or not).

      In my experience, developers tend to then make the mocks a scapegoat, rather than address the habitual coupling in their coding style.

      • Anonymous

        “Developers tend to … make the mocks a scapegoat, rather than address the habitual coupling in their coding style.”

        I think this is a powerful insight and very often true. Mocks are a wonderful tool for exposing bad object design. Unfortunately, this is a double-edged sword: trying to bring badly-designed code under test using mock objects, or refactor badly-designed code when its test suite makes heavy use of mocks, becomes extremely painful.

        For that reason, and for the fact that aesthetically and practically I dislike that mocks require assertion-creating code to be mixed up with arrangement code, I favour test spies over mocks. I do appreciate, though, that not using strict mocks removes one coercion toward good object design.

        • David Doolin

          “Mocks are a wonderful tool for exposing bad object design.”

          Once I started using mocks, I found this as well. Good design is easy to mock. Bad design, not so easy to mock. This became clear to me when mocking own of my own classes which I *knew* was poorly designed.

          On a related note, a little mocking goes a long way.

    • Christopher Johnston

      If you don’t stub, or mock, out your models, how do you stop them from accessing the database? In addition, how do you deal with your unit tests actually becoming integration tests?

      I like to stub everything out and use unit tests to test a single method in isolation from its dependencies. This also solves the problem of accessing the database for every unit test. The project I am on right now, the team is against stubs and mocks, for the reason you stated, and all of our unit tests are actually little integration tests in which we need to build the entire object graph we wish to test. This means that our blueprint file is huge and we have a lot of setup steps that need to be implemented for every test. Most of this is encapsulated behind factories, but it still involves creating a lot of objects that may or may not be used.

      • http://railscasts.com Ryan Bates

        I don’t prevent my unit tests from accessing the database. In that sense I agree they aren’t true unit tests because they are not in complete isolation, but I honestly don’t care. I just care that the tests check that the app works properly.

        I find it’s easy for bugs to slip through the cracks when using stubs everywhere. You can rename a method, and if you aren’t disciplined enough to fix every stub for that method you will have passing tests which should actually fail. To prevent this you need very extensive integration-level tests which not only duplicate the checking that the unit test does, but are often slower than if you would not have used stubs to begin with. So why not cut out the isolated, stubbed unit tests and just stick to the integration tests which actually test the true functionality.

        • http://avdi.org Avdi Grimm

          I appreciate that you recognize that what you are doing isn’t Unit Testing. I think it does a disservice to the industry, and especially people new to agile methods, when people call integration tests “Unit Tests”.

          As for why I still do Unit Tests: It’s been said a zillion times, but I’ll say it again: TDD is a design discipline. The fact that you get a regression suite out of it is a bonus. I do TDD to drive out the simplest design that could possibly work. At the Unit level, verification is gravy.

          At this point I’ve seen so many organizations hobbled by poorly-decoupled code as a result of not having a true TDD Unit-Testing discipline, and struggling as a result to make even small changes to their feature set, or struggling to get the developers to consistently run a test suite that takes 30 minutes to run and frequently breaks, that I feel pretty confident strict TDD at the unit level is still a good idea.

          • http://railscasts.com Ryan Bates

            I agree that TDD is a design discipline, but I don’t believe you need to stub out everything to accomplish this. I once asked Kent Beck, the founder of TDD, how often he does mocking. He responded with “maybe once a year”.

            https://twitter.com/#!/kentbeck/status/39773461270372352

            Some other interesting tweets from him on the subject:

            https://twitter.com/#!/kentbeck/status/21763908817649664
            https://twitter.com/#!/kentbeck/status/18314847410196480

          • http://avdi.org Avdi Grimm

            Thanks for the references! I’ve had similar situations where I had
            acceptance, integration, or functional specs which I felt were adequate.
            Actually, my workflow these days is to write the acceptance test, and only
            drive down to functional or unit level if there’s anything interesting
            (read: logic that would be painful to fully drive-out at the acceptance
            level) at the lower levels.

            Rails, and especially ActiveRecord, is an interesting beast because by
            failing to make any meaningful distinction between domain modeling and
            persistence concerns, it’s particularly easy to write functional or
            integration tests when all you really need is a unit test that specifies
            some business logic. I try to be disciplined about drawing the distinction,
            and I think it pays off both in terms of faster test suites and in terms of
            designs that separate business logic from persistence details.

          • http://railscasts.com Ryan Bates

            I also start at the acceptance/integration level and only go to a lower level when there is complex logic with multiple branches, so perhaps our testing styles are not as different as I had originally thought. :)

            I too try to avoid using the database in tests when possible, but do so by building records in memory through factories and not stubs.

            Thanks for the discussion Avdi, it’s great to learn about different testing styles.

    • http://lucashungaro.github.com/ Lucas Hungaro

      I used to think *exactly* that about stubs, mocks and doubles in general. Then I took some time to try to understand its roles into OO design and I’m glad I did it. Doubles are tools to enforce decoupling and are awesome at that.

      If used incorrectly, yes, they can do more harm than good. Most people actually do not understand the concept (I didn’t for a long time) and just think of doubles as a way to “avoid the db”, “hide dependencies” and things like that.

      To avoid writing a really long comment here, I can point to this post by James Golick as a good sample of “real” double usage: http://jamesgolick.com/2010/3/10/on-mocks-and-mockist-testing.html 

  • http://mwilden.blogspot.com Mark Wilden

    Naturally, I’m still not completely convinced. :) Still, a great article, as I always expect from this blog. The one thing I’ll say is that by using delegation you introduce dependencies that didn’t exist before. Take the department_name example. Before, the user had to know only that he had a department. He didn’t have to know anything else about the department. Now, both the user and the department have to know that a department has a name (and multiply this by the number of public methods on the department). I don’t think that completely vitiates your argument, but did feel it was worth pointing out.

  • http://twitter.com/andrzejkrzywda Andrzej Krzywda

    You may want to have a look at DCI, which solves some of the problems with delegation in OOP.

    Some example of DCI with Ruby are in my article here:
    http://andrzejonsoftware.blogspot.com/2011/02/dci-and-rails.html

  • http://mwilden.blogspot.com Mark Wilden

    This just occurred to me. Because I think “unit” testing is important, too. But I don’t need to unit test subobjects’ data. I just need to test their behavior. So it would make sense to me to mock out behavior, but not data. If I just need the subobjects’ data, I would use a factory. And, even though I wouldn’t ask a customer’s wallet for a payment, I certainly would ask it for its color. I would not write a wallet_color method on the customer. Would anyone?

    • Steve Conover

      This is the problem with a simplistic / robotic application of demeter – you get a combinatorial explosion of strange demeter-satisfying methods in classes where they do not belong.

      Demeter is a pretty good principle, it’s good food for thought, but shouldn’t be taken to its logical conclusion, especially when simply reading data. I will confess to having an initial negative reaction to invocations of demeter – I’ve seen the outcome of simplistically applied demeter (demeter method spam) and it is not pretty. And at some point I think an idea can be blamed for its adherents.

  • http://twitter.com/davetroy Dave Troy

    Seems to me the Law of Demeter is a kind of ethic: a value system to which you aspire. I think the topic of code ethics is one that’s well worth exploring.

    • http://avdi.org Avdi Grimm

      An astute point; I’m putting together notes for a talk that explores exactly this idea.

  • http://twitter.com/mfeathers Michael Feathers

    Great blog, Advi.  Just wanted to pipe in to say a couple of things.

    Demeter can be a real can of worms.  The thing you did with delegators to get past the Demeter Violation is cool, but we can also argue that by exposing department_name we’r exposing the concept of a department from User and possibly muddling the interface.  I just notice that often when Demeter is violated it takes some re-conceptualization of the problem and a completely different factoring of classes to make things much nicer.  Sadly, it’s hard to do incrementally.

    The other thing about Demeter is that some violations are okay (to me).  The criteria that I use is: does the structure have a meaning and is there no other way to encode that meaning?  An example of this sort of thing would be the DOM or parse trees from a compiler.  We get rid of Demeter violations in those places by introducing query and modification languages, but the user still needs to aware of the underlying structure because it has meaning.  And, once that is true, there are still dependency issues.

    The final thing I was going to say is that I like your observation about types being a big deal.  I know that, recently, when I am programming in a functional language, I’m constantly looking at the types and trying to find a kernel of uses of a type that make it easy to chain functions.  For example, if I have 3 functions which each take a different type and return the same type A, then I can use them as input to a chain. If I have functions which take an A to A, those can be part of the chain.  If I have a function which takes an A to a B and a function which takes a B to an A, I can chain those easily also.  It’s sort of like `type attractors.’  I’d argue that when we make DSLs, they are good to the degree that we have those.

  • Anonymous

    Another good article Avdi but might I suggest that the use of delegate from the User class is, in some ways, tightly coupling your User to the Department model. If the implementation of Department were to change then everywhere you have such a call would need to change. Instead it’s better to move this into a module that gets included:

    class User
    include Department::Relationship # For want of a better name
    end
    module Department::Relationship
    def self.included(base)
    base.delegate :name, :to => :department, :prefix => true, :allow_nil => true
    end
    end

    This places knowledge about the Department close to its implementation, and only requires classes that participate in a relationship to Department to include that module. If the name of a Department instance comes from #foobar then you change in one place (in that module), and everything continues on as normal. More importantly, you only need to test that your class includes the Department::Relationship module, and test the behaviour of the module itself once.

    To me, that is also reducing the structural coupling.

    • http://twitter.com/fjfish Francis Fish

      I really don’t like this – you’re hiding your intentions in the include for some kind of ideological purity. This kind of thing can get pathological and you can’t see what the code is supposed to be doing – and you’ve written more code than you needed to. I like it when the code says what it supposed to do in the place it does it.

      A lot of Rubyists are ‘include obsessed’ and don’t use inheritance properly, hiding core responsibilities off the track in a library somewhere. It gets in the way. Include is for cross class or extending, not for basic functionality.It’s almost a monkey patch and adds nothing but obscurity. 

      Plus – have you ever used the include functionality for Rspec? It works, but can be really painful chasing down the dependencies, but ymmv.

  • dkubb

    This is a great post Avdi, but I have a question about the following point you made:

    “There’s nothing wrong with having a large API, so long as individual collaborators only talk to well-defined subsets of it.”

    I’m not sure if you mean API, as in the interface for all the classes and methods in a system, or for just one specific class. I think when you have a class with a large API, it’s a sign it’s doing too much and possibly an SRP violation. I would consider breaking up such a class into smaller classes that handle one responsibility each.

  • Anonymous

    Great article Advi! Like the arguments posted before, I agree that one object knowing about anothers object’s interface is disputable. More importantly, that doesn’t (relatively) couple. So user.department.name is fine.

    The main argument for delemeter is that given you have a transitive relation ship between a, b and c, if  therefor a knows about c, then the relationship is dependent on b. And any change in c would imply changes in b and a. Thats delemter :-)

  • http://ryan.sandridge.org/ Ryan

    Great article Avdi! Apologies if already mentioned… but ActiveRecord can clearly take something away from this article… customer.orders.build({}) is a violation of Demeter, no? As are many of the association methods. I suppose that should be delegated to customer.build_order({}), right?

  • http://www.facebook.com/kripet Kristján Pétursson

    Thanks for the discussion, Advi. There’s just one showstopping bug.

    4. We run the tests again, and this time they pass.
    5. The final step is to implement the User#department_name method.

    Your tests have gone green, but you have yet to build the feature. This is fundamentally broken.

    • Justin Blake

      This is why starting with acceptance or integration tests are important. Once his unit tests are green, the higher-level tests will catch that it hasn’t been implemented. From the article: “…we make a mental note to implement it. If we forget, the omission will be caught by our acceptance and/or integration tests.”

    • http://avdi.org Avdi Grimm

      What Justin Blake said.

  • http://www.facebook.com/kripet Kristján Pétursson

    Thanks for the discussion, Advi. There’s just one showstopping bug.

    4. We run the tests again, and this time they pass.
    5. The final step is to implement the User#department_name method.

    Your tests have gone green, but you have yet to build the feature. This is fundamentally broken.

  • Kelly Stannard

    Sorry for the necromancy here, but I want to say I have an incredible dislike for complex or multi-line ternary operations and using ternary operations as part of a bigger logic operation. It is very easy to skim over the ternary symbols and not notice them and then you have to double back and re-read the section 5 times to figure out what is going on.

    My other comment is that I think ternary part should have been in the country_stats method. But other than those two nitpicks this was a fantastic article and is really going to help me in the future.

  • http://www.facebook.com/noahdavis Noah Davis

    Saw your reference to this post on the parley list. Thanks for this. I’ve always had a discomfort with the “strawman” definition but never did any kind of investigation, so one of this depth is very much appreciated.

  • tibbon

    So I’m working to help refactor an app a bit.
    In some of my bigger models (like User), I suddenly need like 30 delegates.

    And I’m also unsure how to best deal with longer chains that have been setup like User.twitter_account.metrics.best

    Any thoughts on how to deal with that?

    • http://avdi.org Avdi Grimm

      Any particular reason a given method has to descend all the way down that tree from the top level? Or could ‘metrics’ be passed into it from somewhere else?

  • Brandon Weaver

    Typo: s/prfectly/perfectly. Found under section ‘Objection #2′, paragraph 6 (unless you count the block quote as one), sentence 1.