The Law of Demeter is pretty simple:
A method of an object should invoke only the methods of the following kinds of objects:Essentially it means that in languages like Java you should not chain together method calls using the dot operator. A great example of this is detailed in the paper, The Paperboy, The Wallet,
1. itself
2. its parameters
3. any objects it creates/instantiates
4. its direct component objects
and The Law Of Demeter by David Bock. Bock goes through a perfect illustration of why the Law of Demeter should be followed, with an emphasis on scenarios where not using it can cause null pointer exceptions. Sadly, this is the exact case I ran into recently that would have been avoided if I had followed the law.
I had a client interface that needed to get information contained in an object returned from a different method. The needed information was always to be in the first item in a collection, so I immediately used dot operator chaining to get the iterator for the collection and then the first item in the collection.
Item i = (Item)object.methodReturningCollection().iterator().next();
Looking at that code now I see how naive I was being when I wrote it. I never thought of the scenario where the initially called method would return as null . Of course this was caught in testing, but was fixed by simply using a temporary variable and testing for null. This fixes the problem with the null object, but still goes against the law. What I should have done was to create a method in the called object that returned the information I was trying to obtain in the client code. Then the client would not need to know how to retrieve the information as it should be the responsibility of the called object.
I consider myself a fairly decent developer, but you learn something new everyday. I will now be looking out for this when I am doing refactoring. It's amazing to me how much software development is an ongoing learning experience. I love every minute of it...
In his "Paperboy" article, David Block says:
ReplyDeleteThe above license example would be no better if the code looked like,
Wallet tempWallet = person.getWallet();
license = tempWallet.getDriversLicense();
It is equivalent, but harder to detect.
So while using a temporary variable fixed the bug, it did not actually fix the LoD violation -- if indeed there was one.
I would argue that there is no LoD violation in your code, since you are accessing methods of:
* A a parameter
* A collection class (utility)
* An iterator class (utility)
These are all things that your code has the right to access, according to the law. As Phil Haak says, it's not about counting dots. It's about encapsulation and abstraction.
Thanks for the comment. I see your point and agree that I likely didn't violate the law (which really shouldn't be classified as a law in my opinion). However from a maintenance perspective I wish I had made what I was attempting to do with the object from in the client code a method in the parent class that contained the collection.
ReplyDeleteI had to do the same actions against the child object in different parts of the code so I had to go back and fix the code in multiple places. That should have been a red flag that I needed to extract the code into the method, but I guess I let management pressure to get it done cloud my vision. Either way it is a learning experience.
I am going to try and post more design/code topics in the future and I will always welcome your opinion.