How to solve the violations of the Law of Demeter?

Question

A colleague and I 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

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.

Answer 2

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 3

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 4

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 5

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 complete 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 6

I was struggling with the LoD just like many of you were until I watched The Clean Code Talks" session called:

"Don't Look For Things"

The video helps you use Dependency Injection better, which inherently can fix the problems with LoD. By changing your design a bit, you can pass in many lower level objects or subtypes when constructing a parent object, thus preventing the parent from having to walk the dependency chain through the child objects.

In your example, you would need to pass in AdministrationRoute to the constructor of ProtocolMedication. You'd have to redesign a few things so it made sense, but that's the idea.

Having said that, being new to the LoD and no expert, I would tend to agree with you and DrJokepu. There are probably exceptions to the LoD like most rules, and it might not apply to your design.

[ Being a few years late, I know this answer probably won't help the originator, but that's not why I'm posting this ]