Law of Demeter violation proves useful. Am I missing something? [closed]_问答_开发者

Closed. This question is opinion-based. It is not currently accepting answers.

Want to improve this question? Update the question so it can be answered with facts and citations by editing this post.

Closed 4 years ago.

Improve this 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.

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.

I think it's difficult to give you a clear-cut answer because

1) the problem statement is pretty abstract, and
2) there is no "absolute" good design - it depends on what's around your classes too, and what was a good design initially might evolve into something you want to refactor as your system grows and evolves, and your understanding of the domain becomes more refined.

I don't see the first example as a "massive" violation of the Demeter principle, but again everything is in the details, it depends on how much is going on in your commented section - you can always add more indirection if you need to. You could for instance have your method "DoStuff" on a WriteBusinessObjectToXmlService class, and if the amount of work you were doing involving f was growing, you could extract it into its method "DoStuffWithF(f, x)", or even create a separate class WriteFToXmlService, with DoStuff(f, x).

If we follow with this logic further we will come up with an idea that a global-everything-objects-repository object (or service locator) should be used which contains links to everything in the system. Than we will not need to change method signatures at all because this repository is all we need.

The problem is that the purpose of the method has changed, but the signature has not. If Foo is everything the method needs than it should accept Foo only. This way we can tell that it operates solely on Foo. This will communicate the purpose of the method more clearly. If it suddenly needs Business too we need to change the method signature because it should indicate other method purpose and requirements

Maybe it is now justified to pass in Business or the method needs a third parameter: the return type of the other method called on the Business object. It depends on the rest of the body of the doStuff method.