Reduce coupling and improve encapsulation…
General
In this post I want to go over Law of Demeter (LoD). I find this topic an extremely important for having the code clean, well-designed and maintainable.
In my experience, seeing it broken is a huge smell for bad design. Following the law, or refactoring based on it, leads to much improved, readable and more maintainable code.
So what is Law of Demeter?
I will start by mentioning the 4 basic rules:
Law of Demeter says that a method M of object O can access / invoke methods of:
- O itself
- M’s input arguments
- Any object created in M
- O’s parameters / dependencies
These are fairly simple rules.
Let’s put this in other words: Each unit (method) should have limited knowledge about other units.
Metaphors
The most common one is: Don’t talk to strangers
How about this:
Suppose I buy something at 7-11. When I need to pay, will I give my wallet to the clerk so she will open it and get the money out? Or will I give her the money directly?
How about this metaphor:
When you take your dog out for a walk, do you tell it to walk or its legs?
Why do we want to follow this rule?
- We can change a class without having a ripple effect of changing many others.
- We can change called methods without changing anything else.
- Using LoD makes our tests much easier to construct. We don’t need to write so many ‘when‘ for mocks that return and return and return.
- It improves the encapsulation and abstraction (I’ll show in the example below).
But basically, we hide “how things work”. - It makes our code less coupled. A caller method is coupled only in one object, and not all of the inner dependencies.
- It will usually model better the real world.
Take as an example the wallet and payment.
Counting Dots?
Although usually many dots imply LoD violation, sometimes it doesn’t make sense to “merge the dots”.
Does:
getEmployee().getChildren().getBirthdays()
suggest that we do something like:
getEmployeeChildrenBirthdays()
I am not entirely sure.
Too Many Wrapper Classes
This is another outcome of trying to avoid LoD. In this particular situation, I strongly believe that it’s another design smell which should be taken care of.
As always, we must have common sense while coding, cleaning and / or refactoring.
Example
Suppose we have a class: Item. The item can hold multiple attributes. Each attribute has a name and values (it’s a multiple value attribute)
The simplest implementations would be using Map.
public class Item {
private final Map<String, Set<String>> attributes;
public Item(Map<String, Set<String>> attributes) {
this.attributes = attributes;
}
public Map<String, Set<String>> getAttributes() {
return attributes;
}
}
Let’s have a class ItemsSaver that uses the Item and attributes: (please ignore the unstructured methods. This is an example for LoD, not SRP)
public class ItemSaver {
private String valueToSave;
public ItemSaver(String valueToSave) {
this.valueToSave = valueToSave;
}
public void doSomething(String attributeName, Item item) {
Set<String> attributeValues = item.getAttributes().get(attributeName);
for (String value : attributeValues) {
if (value.equals(valueToSave)) {
doSomethingElse();
}
}
}
private void doSomethingElse() {
}
}
Suppose I know that it’s a single value (from the context of the application). And I want to take it. Then the code would look like:
Set<String> attributeValues = item.getAttributes().get(attributeName); String singleValue = attributeValues.iterator().next(); // String singleValue = item.getAttributes().get(attributeName).iterator().next();
I think that it is clear to see that we’re having a problem. Wherever we use the attributes of the Item, we know how it works. We know the inner implementation of it. It also makes our test much harder to maintain.
Let’s see an example of a test using mock (Mockito): You can see imagine how much effort it should take to change and maintain it.
Item item = mock(Item.class);
Map<String, Set<String>> attributes = mock(Map.class);
Set<String> values = mock(Set.class);
Iterator<String> iterator = mock(Iterator.class);
when(iterator.next()).thenReturn("the single value");
when(values.iterator()).thenReturn(iterator);
when(attributes.containsKey("the-key")).thenReturn(true);
when(attributes.get("the-key")).thenReturn(values);
when(item.getAttributes()).thenReturn(attributes);
We can use real Item instead of mocking, but we’ll still need to create lots of pre-test data.
Let’s recap:
- We exposed the inner implementation of how Item holds Attributes
- In order to use attributes, we needed to ask the item and then to ask for inner objects (the values).
- If we ever want to change the attributes implementation, we will need to make changes in the classes that use Item and the attributes. Probably a-lot classes.
- Constructing the test is tedious, cumbersome, error-prone and lots of maintenance.
Improvement
The first improvement would be to ask let Item delegate the attributes.
public class Item {
private final Map<String, Set<String>> attributes;
public Item(Map<String, Set<String>> attributes) {
this.attributes = attributes;
}
public boolean attributeExists(String attributeName) {
return attributes.containsKey(attributeName);
}
public Set<String> values(String attributeName) {
return attributes.get(attributeName);
}
public String getSingleValue(String attributeName) {
return values(attributeName).iterator().next();
}
}
And the test becomes much simpler.
Item item = mock(Item.class);
when(item.getSingleValue("the-key")).thenReturn("the single value");We are (almost) hiding totally the implementation of attributes from other classes.
The client classes are not aware of the implementation expect two cases:
- Item still knows how attributes are built.
- The class that creates Item (whichever it is), also knows the implementation of attributes.
The two points above mean that if we change the implementation of Attributes (something else than a map), at least two other classes will need to be change. This is a great example for High Coupling.
The Next Step Improvement
The solution above will sometimes (usually?) be enough. As pragmatic programmers, we need to know when to stop. However, let’s see how we can even improve the first solution.
Create a class Attributes:
public class Attributes {
private final Map<String, Set<String>> attributes;
public Attributes() {
this.attributes = new HashMap<>();
}
public boolean attributeExists(String attributeName) {
return attributes.containsKey(attributeName);
}
public Set<String> values(String attributeName) {
return attributes.get(attributeName);
}
public String getSingleValue(String attributeName) {
return values(attributeName).iterator().next();
}
public Attributes addAttribute(String attributeName, Collection<String> values) {
this.attributes.put(attributeName, new HashSet<>(values));
return this;
}
}
And the Item that uses it:
public class Item {
private final Attributes attributes;
public Item(Attributes attributes) {
this.attributes = attributes;
}
public boolean attributeExists(String attributeName) {
return attributes.attributeExists(attributeName);
}
public Set<String> values(String attributeName) {
return attributes.values(attributeName);
}
public String getSingleValue(String attributeName) {
return attributes.getSingleValue(attributeName);
}
}
(Did you noticed? The implementation of attributes inside item was changed, but the test did not need to. This is thanks to the small change of delegation.)
In the second solution we improved the encapsulation of Attributes. Now even Item does not know how it works.
We can change the implementation of Attributes without touching any other class.
We can make different implementations of Attributes:
- An implementation that holds a Set of values (as in the example).
- An implementation that holds a List of values.
- A totally different data structure that we can think of.
As long as all of our tests pass, we can be sure that everything is OK.
What did we get?
- The code is much more maintainable.
- Tests are simpler and more maintainable.
- It is much more flexible. We can change implementation of Attributes (map, set, list, whatever we choose).
- Changes in Attribute does not affect any other part of the code. Not even those who directly uses it.
- Modularization and code reuse. We can use Attributes class in other places in the code.
