I’ve been doing some more reading and came across the Law of Demeter and train wrecks again. The last time while while reading Clean Code.
I’ve one book that tells me this is bad practice:
const auto path = context.GetOptions().GetScratchDirectory().GetAbsolutePath();
And that I should do this instead:
const auto options = context.GetOptions(); const auto directory = options.GetScratchDirectory(); const auto path = directory.getAbsolutePath();
My feeling was to prefer the first as more compact here and it avoids turning a simple context wrapper into a heavyweight class. Now another book that tells me that both are bad. It doesn’t feel like there was too much to back either up. I’ll dig in to it.
Loose coupling and the law
The law is a specific case of loose coupling. The general goal with loose coupling is to ensure that components make use of little or no internal knowledge about other components. That allows each component to be changed with little or no impact on other components. With Demeter it’s specifically about a method M within a class C and what it can interact with:
- Other instances methods in C
- The parameters of M
- Methods in objects that it creates
- Global variables
Roughly speaking that means we should not reach through a series of other methods or classes to do something because that requires knowledge of each system along the way. This was initially devised in the 1980s and so global variables were more of a thing. That’s likely to be discounted today.
The first code sample with method call after method call gets called train wrecks because they look like a bunch of coupled train cars. It’s considered bad because it links the calling code to code several steps away. The second code sample is also considered bad for the same reason, it is just spread out over more lines.
Clean Code notes that this is particular to classes and method calls so that access structures is considered okay:
const auto path = context.options.scratchDirectory.absolutePath;
On the face of it loose coupling makes sense. One component should be able to use another without detailed knowledge of it’s internals. Ideally an interface should make certain guarantees about what it provides but not how it provides them. This law allows the set of methods callable directly from the start point but nothing else. You could not make components any less connected than this. However technically that could still involve many things. It all depends on how many parameters and other instance methods there are. To me it ends up feeling somewhat arbitrary. Many things have pros and cons and going to one extreme may not be right.
Wikipedia claims that it makes code that is more maintainable and adaptable but without references. It goes on to suggest that calling a method with invoke fewer sub-methods but each class will end up with more methods, these decrease and increase the probability of bugs respectively. So that seems to be a wash as well.
On balance
While loose coupling sounds good I can’t say the same about the specifics of the Law of Demeter.
Most of the examples I’ve seen shows a “train wreck” which is being used to access a relative slim set of interfaces. After they’ve “fixed it” the interfaces are much bulkier and the intermediate level is full of code that just exists pass things down to lower levels. It does mean that you can swap out the lower level and only have to change one file… Is that even a great benefit if you have ownership of all levels?
For me it feels like how many method calls are in a chain might be a distraction. I think it’s better to worry about whether classes and methods have distinct responsibilities instead. Classes encapsulate data and are responsible for it. That doesn’t just mean containing that data but performing the necessary calculations on it as well. In that case more of the information is likely to be local and therefore there will be few or smaller chains, reducing coupling.
If you do have a chain consider what it does and where it goes. Some classes cluster together and it feels okay that they should be talking to each other. Other classes are simple wrappers around read-only data, such as the options
above, and increase coupling but are unlikely to cause bugs. If you have a chain that changes the state of other objects that is more likely to cause problems. All the more likely if that object is in some sense distant. Is it on a different server, manage by a different team or company? Anywhere there is an interface between systems it does make sense to slow down and think about them again.
Leave a Reply