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

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 2

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 3

The third problem is very simple: Discipline.ToString() should evaluate the Name property That way you only call Kind.Discipline

Answer 4

I think it helps to remember the raison d'être of LoD. That is, if details change in chains of relationships, your code could break. Since the classes you have are abstractions close to the problem domain, then the relations aren't likely to change if the problem stays the same, e.g., Protocol uses Discipline to get its work done, but the abstractions are high level and not likely to change. Think of information hiding, and it's not possible for a Protocol to ignore the existence of Disciplines, right? Maybe I'm off on the domain model understanding...

This link between Protocol and Discipline is different than "implementation" details, such as order of lists, format of data structures, etc. that could change for performance reasons, for example. It's true this is a somewhat gray area.

I think that if you did a domain model, you'd see more coupling than what is in your C# class diagram. [Edit] I added what I suspect are relationships in your problem domain with dashed lines in the following diagram:

On the other hand, you could always refactor your code by applying the Tell, don't ask metaphor:

That is, you should endeavor to tell objects what you want them to do; do not ask them questions about their state, make a decision, and then tell them what to do.

You already refactored the first problem (BLL) with your answer. (Another way to abstract BLL further would be with a rule engine.)

To refactor the second problem (repositories), the inner code

    p.Kind.Discipline.Id == discipline.Id

could probably be replaced by some kind of .equals() call using a standard API for Collections (I'm more of a Java programmer, so I'm not sure of the precise C# equivalent). The idea is to hide the details of how to determine a match.

To refactor the third problem (inside the UI), I'm also not familiar with ASP.NET, but if there's a way to tell a Kind object to return the names of Disciplines (rather than asking it for the details as in Kind.Discipline.Name), that's the way to go to respect LoD.