How to solve the violations of the Law of Demeter?

Question

Me and a colleague designed a system for our customer, and in our opinion we created a nice clean design. But I'm having problems with some coupling we've introduced. I could try to create an example design which includes the same problems as our design, but if you forgive me I'll create an extract of our design to support the question.

We're developing a system for the registration of certain treatments for a patients. To avoid having a broken link to image I'll describe the conceptual UML class diagram as a c# style class definition.

class Discipline {}
class ProtocolKind 
{ 
   Discipline; 
}
class Protocol
{
   ProtocolKind;
   ProtocolMedication; //1..*
}
class ProtocolMedication
{
   Medicine;
}
class Medicine
{
   AdministrationRoute;
}
class AdministrationRoute {}

I'll try to explain a bit about the design, a protocol is the template for a new treatment. And a protocol is of a certain Kind and has medications which need to be administered. Per protocol the dosage can differ for the same medicine (amongst other things), so that's stored in the ProtocolMedication class. AdministrationRoute is the way the medicine is administered and iscreated/updated separately from the protocol management.

I've found the following places which we'll have a violation of the Law of Demeter:

Violations of the Law of Demeter

Inside of the BLL

For example, inside the business logic of the ProtocolMedication there are rules which depend on the AdministrationRoute.Soluble property of the medicine. The code would become

if (!Medicine.AdministrationRoute.Soluble)
{
   //validate constrains on fields
}

Inside the repositories

A method which would list all the protocols in a certain Discipline would be written as:

public IQueryable<Protocol> ListQueryable(Discipline discipline)
{
    return ListQueryable().Where(p => (p.Kind.Discipline.Id == discipline.Id)); // Entity Frameworks needs you to compare the Id...
}

Inside the User Interface

We use ASP.NET (no MVC) for the interface of our system, in my opinion this layer currently has the worst violations. The databinding of a gridview (a column which must display the Discipline of a protocol must bind to Kind.Discipline.Name), which are strings, so no compile time errors.

<asp:TemplateField HeaderText="Discipline" SortExpression="Kind.Discipline.Name">
   <ItemTemplate>
      <%# Eval("Kind.Discipline.Name")%>
   </ItemTemplate>
</asp:TemplateField>

So I think the actual question might be, when would it be okay to look at it more as the Suggestion of Demeter, and what can be done to solve the violations of the Law of Demeter?

I've got a few idea's of myself but I'll post them as answers so they can be commented and voted on seperatly. (I'm not sure this is the SO way to do it, if not, I'll delete my answers and add them to the question).

Answer 1

For the BLL my idea was to add a property on Medicine like this:

public Boolean IsSoluble
{
    get { return AdministrationRoute.Soluble; } 
}

Which is what I think is described in the articles about Law of Demeter. But how much will this clutter the class?

Answer 2

For the user interface I was thinking of creating an extra service layer which returned "flat" value classes only suited for display of entity information.

But how should I name these result classes?

Answer 3

I know I'm going to get downvoted to total annihilation but I must say I sort of dislike Law of Demeter. Certainly, things like

dictionary["somekey"].headers[1].references[2]

are really ugly, but consider this:

Kitchen.Ceiling.Coulour

I have nothing against this. Writing a bunch of functions just to satisfy the Law of Demeter like this

Kitchen.GetCeilingColour()

just looks like a total waste of time for me and actually gets is my way to get things done. For example, what if the requirements change and I will need the ceiling height too? With the Law of Demeter, I will have to write an other function in Kitchen so I can get the Ceiling height directly, and in the end I will have a bunch of tiny getter functions everywhere which is something I would consider quite some mess.

EDIT: Let me rephrase my point:

Is this level of abstracting things so important that I shall spend time on writing 3-4-5 levels of getters/setters? Does it really make maintenance easier? Does the end user gain anything? Is it worth my time? I don't think so.

Answer 4

Concerning the first example with the "soluble" property, I have a few remarks:

1. What is "AdministrationRoute" and why would a developer expect to get a medicine's soluble property from it? The two concepts seem entirely unrelated. This means the code does not communicate very well and perhaps the decomposition of classes you already have could be improved. Changing the decomposition could lead you to see other solutions for your problems.
2. Soluble is not a direct member of medicine for a reason. If you find you have to access it directly then perhaps it should be a direct member. If there is an additional abstraction needed, then return that additional abstraction from the medicine (either directly or by proxy or façade). Anything that needs the soluble property can work on the abstraction, and you could use the same abstraction for multiple additional types, such as substrates or vitamins.

Answer 5

Instead of going all the way and providing getters/setters for every member of every contained object, one simpler alteration you can make that offers you some flexibility for future changes is to give objects methods that return their contained objects instead.

E.g. in C++:

class Medicine {
public:
    AdministrationRoute()& getAdministrationRoute() const { return _adminRoute; }

private:
    AdministrationRoute _adminRoute;
};

Then

if (Medicine.AdministrationRoute.Soluble) ...

becomes

if (Medicine.getAdministrationRoute().Soluble) ...

This gives you the flexibility to change getAdministrationRoute() in future to e.g. fetch the AdministrationRoute from a DB table on demand.

Answer 6

I'd have to assume that the business logic that requires Soluble requires other things too. If so, can some part of it be incapsulated in Medicine in a meaningful way (more meaningful than Medicine.isSoluble())?

Another possibility (probably an overkill and not comlpete solution at the same time) would be to present the business rule as object of its own and use double dispatch/Visitor pattern:

RuleCompilator
{
  lookAt(Protocol);
  lookAt(Medicine);
  lookAt(AdminstrationProcedure) 
}

MyComplexRuleCompilator : RuleCompilator
{
  lookaAt(Protocol)
  lookAt(AdminstrationProcedure)
}

Medicine
{
  applyRuleCompilator(RuleCompilator c) {
    c.lookAt(this);
    AdministrationProtocol.applyRuleCompilator(c);
  }
}

Answer 7

My understanding of consequences of the Law of Demeter seems to be different to DrJokepu's - whenever I've applied it to object oriented code it's resulted in tighter encapsulation and cohesion, rather than the addition of extra getters to contract paths in procedural code.

Wikipedia has the rule as

More formally, 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:

1. O itself
2. M's parameters
3. any objects created/instantiated within M
4. O's direct component objects

If you have a method which takes 'kitchen' as a parameter, Demeter says you cannot inspect the components of the kitchen, not that you can only inspect the immediate components.

Writing a bunch of functions just to satisfy the Law of Demeter like this

Kitchen.GetCeilingColour()

just looks like a total waste of time for me and actually gets is my way to get things done

If a method outside of Kitchen is passed a kitchen, by strict Demeter it can't call any methods on the result of GetCeilingColour() on it either.

But either way, the point is to remove the dependency on structure rather than moving the representation of the structure from a sequence of chained methods to the name of the method. Making methods such as MoveTheLeftHindLegForward() in a Dog class doesn't do anything towards fulfilling Demeter. Instead, call dog.walk() and let the dog handle its own legs.

For example, what if the requirements change and I will need the ceiling height too?

I'd refactor the code so that you are working with room and ceilings:

interface RoomVisitor {
  void visitFloor (Floor floor) ...
  void visitCeiling (Ceiling ceiling) ...
  void visitWall (Wall wall ...
}

interface Room { accept (RoomVisitor visitor) ; }

Kitchen.accept(RoomVisitor visitor) {
   visitor.visitCeiling(this.ceiling);
   ...
}

Or you can go further and eliminate getters totally by passing the parameters of the ceiling to the visitCeiling method, but that often introduces a brittle coupling.

Applying it to the medical example, I'd expect a SolubleAdminstrationRoute to be able to validate the medicine, or at least call the medicine's validateForSolubleAdministration method if there's information encapsulated in the medicine's class which is required for the validation.

However, Demeter applies to OO systems - where data is encapsulated within the objects which operate upon the data - rather than the system you're talking about, which has different layers an data being passed between the layers in dumb, navigatable structures. I don't see that Demeter can necessarily be applied to such systems as easily as to monolithic or message based ones. (In a message based system, you can't navigate to anything which isn't in the grams of the message, so you're stuck with Demeter whether you like it or not)

Answer 8

The traditional solution to Demeter violations is "tell, don't ask." In other words, based on your state, you should tell a managed object (any object you hold) to take some action -- and it will decide whether to do what you ask, depending on its own state.

As a simple example: my code uses a logging framework, and I tell my logger that I want to output a debug message. The logger then decides, based on its configuration (perhaps debugging isn't enabled for it) whether or not to actually send the message to its output devices. A LoD violation in this case would be for my object to ask the logger whether it's going to do anything with the message. By doing so, I've now coupled my code to knowledge of the logger's internal state (and yes, I picked this example intentionally).

However, the key point of this example is that the logger implements behavior.

Where I think the LoD breaks down is when dealing with an object that represents data, with no behavior.

In which case, IMO traversing the object graph is no different than applying an XPath expression to a DOM. And adding methods such as "isThisMedicationWarranted()" is a worse approach, because now you're distributing business rules amongst your objects, making them harder to understand.