I came back to work from vacation yesterday, and in our daily standup, my teammates mentioned they were refactoring all of the model objects in our java code to remove all getters and setters and make the model fields all public objects instead, invoking the Law of Demeter as the reason for doing so because
to facilitate the our adherence to Demeter's law: a module should not know about the innards of the 'objects' it manipulates. Since data
structures contain no behavior, they naturally exposes their internal structure. So in that case, Demeter does not apply.
I admit I had to brush up on my knowledge of the LoD, but for the life of me I can't find anything to indicate that this is within the spirit of the law. None of the getters/setters in our models contain any business logic, which is his justification for doing this so clients of these objects don't have to understand whether or not there is some business logic being executed within the get/set methods.
I think this is a misunderstanding of what it means to need 'internal knowledge of an objects structure', or at the very least is taking it too literally and breaking a pretty standard convention in the process.
So my question is, does it actually make any sense to expose model objects internal structure directly instead of via getters/setters in the name of the LoD?
There is a book called Clean Code by Robert Martin that covers this.
In Chapter 6 (Objects and Data Structures) he talks about fundamental differences between objects and data structures.
Objects benefit from encapsulation, data structures don't.
There is a section about the Law of Demeter:
There is a well-known heuristic called the Law of Demeter that says a module should not know about the innards of the objects it manipulates. As we saw in the last section, objects hide their data and expose operations. This means that an object should not expose its internal structure through accessors because to do so is to expose, rather than to hide, its internal structure.
More precisely, the Law of Demeter says that a method f of a class C should only call the methods of these:
- C
- An object created by f
- An object passed as an argument to f
- An object held in an instance variable of C
The method should not invoke methods on objects that are returned by any of the allowed functions. In other words, talk to friends, not to strangers.
Uncle Bob gives an example of a LoD violation:
final String outputDir = ctxt.getOptions().getScratchDir().getAbsolutePath();
Whether this is a violation of Demeter depends on whether or not ctxt, Options, and ScratchDir are objects or data structures. If they are objects, then their internal structure should be hidden rather than exposed, and so knowledge of their innards is a clear violation of the Law of Demeter. On the other hand, if ctxt, Options, and ScratchDir are just data structures with no behavior, then they naturally expose their internal structure, and so Demeter does not apply.
The use of accessor functions confuses the issue. If the code had been written as follows, then we probably wouldn’t be asking about Demeter violations.
final String outputDir = ctxt.options.scratchDir.absolutePath;
So this is probably where your co-workers are coming from. I think the argument "we have to do this because LoD" is imprecise at best. The central issue isn't LoD so much as whether the API consists of objects or data structures. It does seem like an unnecessary and error-prone change to push through when there are more pressing things to do.
It doesn't seem to me that this change has anything to do with the Law of Demeter. The law is, essentially, about encoding the structure of your object graph into your code by having methods call through an entire chain of other objects. For example, suppose, in a car insurance application, that a customer has a policy, and a policy has vehicles, and vehicles have drivers assigned to them, and drivers have birth dates and, thus ages. You could imagine the following code:
public boolean hasUnderageDrivers(Customer customer) {
for (Vehicle vehicle : customer.getPolicy().getVehicles()) {
for (Driver driver : vehicle.getDrivers()) {
if (driver.getAge() < 18) {
return true;
}
}
}
return false;
}
This would violate the Law of Demeter because this code now has knowledge of internals that it doesn't need to know. It knows that drivers are assigned to vehicles, instead of just being assigned to the insurance policy as a whole. If, in the future, the insurance company decided that drivers would simply be on the policy, instead of being assigned to particular vehicles, then this code would have to change.
The problem is that it calls a method of its parameter, getPolicy(), and then another, getVehicles(), and then another, getDrivers(), and then another, getAge(). The Law of Demeter says that a method of a class should only call methods on:
- Itself
- Its fields
- Its parameters
- Objects it creates
(The last one can be a problem for unit testing, where you may want to have objects injected or created by factories rather than created directly locally, but that's not relevant to the Law of Demeter.)
To fix the problem with hasUnderageDrivers() we can pass in the Policy object and we can have a method on Policy that knows how to determine whether the policy has underage drivers:
public boolean hasUnderageDrivers(Policy policy) {
return policy.hasUnderageDrivers();
}
Calling one level down, customer.getPolicy().hasUnderageDrivers(), is probably okay — the Law of Demeter is a rule of thumb, not a hard-and-fast rule. You also probably don't have to worry much about things that aren't likely to change; Driver is probably always going to continue to have a birth date and a getAge() method.
But returning to your case, what happens if we replace all these getters with public fields? It doesn't help with the Law of Demeter at all. You can still have the exact same problem as in the first example. Consider:
public boolean hasUnderageDrivers(Customer customer) {
for (Vehicle vehicle : customer.policy.vehicles) {
for (Driver driver : vehicle.drivers) {
if (driver.age < 18) {
return true;
}
}
}
return false;
}
(I've even converted driver.getAge() to driver.age, although that would probably be a calculation based on the birth date rather than a simple field.)
Notice that the exact same problem with embedding knowledge of how the object graph is put together (a customer has a policy which has vehicles which have drivers) is present when we write the code with public fields instead of getters. The problem has to do with how the pieces are put together, not with whether getters are being called.
Incidentally, the normal reason to prefers getters over (final?) public fields is that you may need to put some logic behind them later on. An age is replaced with a calculation based on the birth date and today's date, or a setter needs to have some validation (throws if you pass null, for instance) associated with it. I haven't heard the Law of Demeter invoked in that context before.
The Law of Demeter about the working with objects, not data structure, in your case DTO as I understand.
The Law of Demeter explains that you can call methods of objects that are:
- Passed as arguments
- Cleated locally inside the method
- Instance variables (object's fields)
- Global
Data models represent containers with some data inside them that should be shown outside. It's their role and they haven't some other behavior besides this.
