Law of Demeter – Does This Code Violate It?

law-of-demeter

Let's say I have a class SelectableEntity<T extends Entity> which has three methods, select, deselect, isSelected and count.

To take a somewhat contrived example, let's say I'm building an emergency messaging application that allows me to message to residents during fires, floods and other natural disaster.

I can select multiple entities of different types (but all for the same reason). Let's say these entities are House, Street and Suburb.

Now let's say I have a class called MessageRecipients that contains a SelectableEntity<House>, SelectableEntity<Street>, SelectableEntity<Suburb>.

There are two ways I could code this:

The first way is to create getters (i.e. houses, streets, suburbs) and select / deselect recipients that way.: messageRecipients.streets.select(aStreet).

The other way would be to hide the fact that MessageRecipients is using SelectableEntity at all and simple create 'proxy' methods for each method on SelectableEntity (i.e. selectHouse, deselectHouse, selectStreet, etc).

The first method seems like it violates the Law of Demeter whereas the second method basically requires me to create a whole bunch of methods on MessageRecipients that just directly invoke the relevant instance of SelectableEntity (and requires a whole bunch of extra testing to ensure the correct methods are being invoked).

My question is, is the first example a standard example of a violation of the Law of Demeter and would it preferable to stomach all the duplication and verbosity and go down the second approach (a guess a follow question is what constitutes a valid reason to break the Law of Demeter, if anything)?

Note: The example I've given is made-up and I'm aware that it might break down in certain situations (i.e. streets and suburbs contain houses – if I add a street which has house XYZ and I call messageRecipients.houses().isSelect(houseXYZ) it should return true). For the sake of this discussion that case should be ignored as it doesn't apply to the actual scenario I'm dealing with.

Best Answer

There are two ways I could code this:

The first way is to create getters (i.e. houses, streets, suburbs) and select / deselect recipients that way.: messageRecipients.streets.select(aStreet).

Don't think I prefer this way but I don't want to see the Law of Demeter misapplied.

Some think this is all LoD has to say:

the Law of Demeter for functions requires that a method m of an object O may only invoke the methods of the following kinds of objects:

  • O itself
  • m's parameters
  • Any objects created/instantiated within m
  • O's direct component objects
  • A global variable, accessible by O, in the scope of m

In particular, an object should avoid invoking methods of a member object returned by another method. For many modern object oriented languages that use a dot as field identifier, the law can be stated simply as "use only one dot". That is, the code a.b.Method() breaks the law where a.Method() does not. As an analogy, when one wants a dog to walk, one does not command the dog's legs to walk directly; instead one commands the dog which then commands its own legs.

Wikipedia: Law of Demeter

This is a monument to structural thinking. There is not a single semantic or conceptual thought in any of that. A static analyzer could enforce this. Bleh. This is not all I think of when I think of the LoD.

The Law of Demeter is not a dot counting exercise.

When you are an object, it is better to only talk to your friends, not friends of friends. If you randomly pull in anything you happen to need you'll soon find you've pulled together so many things that any change to the code base is going to hurt you right here.

Friends are things whose interfaces you own. Only then are you promised that you could jump through these dots and always find the same thing. While that can happen it's not going to be true if you just randomly grab whatever you like out of a code base.

So if the client calling "messageRecipients.streets.select(aStreet)" owns the interface for messageRecipients and streets then it's talking to its friends and there is no LoD violation. This means the client owns all the interfaces it uses. They should not change without the client's permission. This should be made clear to maintenance programmers. This needs clear boundaries. But it's not about dot counting.

That's the real problem. Because of the way you're getting this other object, the dependence on it's interface isn't explicit. The dependence can be a surprise. If you respect that you'll find some way to make this dependence clear to other coders.

This shouldn't be some cobbled together situation. This should be a situation created by careful design. What's being created here actually has a name. It's called a DSL. If that's what you're making fine. If this is just some randomly thrown together stuff because you happened to need it then you're creating a problem waiting to happen.

The other name the LoD goes by is "the principle of least knowledge". It's asking you to understand that when you create these dot chains you're adding knowledge to your client not only of these interfaces but how they connect. The more you know the more it hurts when things change. Don't create chains no one promised would be stable.

The other way would be to hide the fact that MessageRecipients is using SelectableEntity at all and simple create 'proxy' methods for each method on SelectableEntity (i.e. selectHouse, deselectHouse, selectStreet, etc).

I like this idea, but i'd also hide whether what's being selected is a house, street, or suburb. Let the suburb be the only thing that cares that it's a suburb.

Related Topic