Law of Demeter violation proves useful. Am I missing something?

Question

I have some code like this in my application. It writes out some XML:-

public void doStuff( Business b, XMLElement x)
{
   Foo f = b.getFoo();
   // Code doing stuff with f
   // b is not mentioned again.
}

As I understand it, the Law of Dementer would say this is bad. "Code Complete" says this is increasing coupling. This method should take "f" in the first place.

public void doStuff( Foo f, XMLElement x)
{
    // Code doing stuff with f
}

However, now I have come to change this code, and I do actually need to access a different method on b.

public void doStuff( Business b, XMLElement x)
{
   Foo f = b.getFoo();
   // Code doing stuff with f
   // A different method is called on b.
}

This interface has made life easier as the change is entirely inside the method. I do not have to worry about the many places it is called from around the application.

This suggests to me that the original design was correct. Do you agree? What am I missing?

PS. I do not think the behaviour belongs in b itself, as domain objects do not know about the external representation as XML in this system.

Answer 1

First thing is that this isn't necessarily a Law of Demeter violation, unless you are actually calling methods on the Foo object f in doStuff. If you are not, then you are probably fine; you're only using the interface of the Business object b. So I'll assume that you are calling at least one method on 'f'.

One thing you might be "missing" is testability, specifically unit tests. If you have:

public void doStuff( Business b, XMLElement x)
{
    Foo f = b.getFoo();
    // stuff using f.someMethod
    // business stuff with b
    // presumably something with x
}

... then if you want to test that doStuff does the right thing for different Foo, you have to first create (or mock) a new Business object with each Foo 'f' that you want, then plug that object into doStuff (even if the rest of the Business-specific stuff is identical). You're testing at one remove from your method, and while your source code may stay simple, your test code gets messier. So, if you really need both f and b in doStuff, then arguably they should both be parameters.

For more reading on it, this person is one of the most emphatic Law of Demeter campaigners I've come across; frequently provides rationales for it.