Comment by Chris_Newton
5 years ago
Talk to directly linked objects and tell them what you need done, and let them deal with their linked objects. Don't assume that you know what is and always will be involved in doing something on dependent objects of the one you're interacting with.
IMHO, this is one of those ideas you have to consider on its merits for each project.
My own starting point is usually that I probably don’t want to drill into the internals of an entity that are implementation details at a lower level of abstraction than the entity’s own interface. That’s breaking through the abstraction and defeating the point of having a defined interface.
However, there can also be relationships between entities on the same level, for example if we’re dealing with domain concepts that have some sort of hierarchical relationship, and then each entity might expose a link to parent or child entities as part of its interface. In that case, I find it clearer to write things like
if (entity.parent.interesting_property !== REQUIRED_VALUE) {
abort_invalid_operation();
}
instead of
let parent_entity = entities.find(entity.parent_id);
if (parent_entity.interesting_property !== REQUIRED_VALUE) {
abort_invalid_operation();
}
and this kind of pattern might arise often when we’re navigating the entity relationships, perhaps finding something that needs to be changed and then checking several different constraints on the overall system before allowing that change.
The “downside” of this is that we can no longer test the original entity’s interface in isolation with unit tests. However, if the logic requires navigating the relationships like this, the reality is that individual entities aren’t independent in that sense anyway, so have we really lost anything of value here?
I find that writing a test suite at the level of the overall set of entities and their relationships — which is evidently the smallest semantically meaningful data set if we need logic like the example above — works fine as an alternative to dogmatically trying to test the interface for a single entity entirely in isolation. The code for each test just sets up the store of entities and adds the specific instances and relationships I want for each test, which makes each test scenario nicely transparent. This style also ensures the tests only depend on real code, not stand-ins like mocks or stubs.
I don't think the two versions are relevant to Law of Demeter. One example has pointers/references in a strong tree and another has indexed ones, but neither is embracing LoD more or less than the other.
This would be a more relevant example:
parent_entity.children.remove(this)
vs
parent_entity.remove_child(this)
...Where remove_child() would handle removing the entity from `children` directly, and also perhaps busting a cache, or notifying the other children that the heirarchy has changed, etc etc.
Going back to your original case, you _could_ argue that LoD would advise you to create a method on entity which returns the parent, but I think that would fall under encapsulation. If you did that though, you could hide the implementation detail of whether `parent` is a reference or an ID on the actual object, which is what most ORMs will do for you.
Ah, but what if children is some kind of List or Collection which can be data-bound? By Liskov's substition principle, you ought to be able to pass it to a Collection-modifying routine and have it function correctly. If the parent must be called the children member should be private, or else the collection should implement eventing and the two methods should have the same effect (and ideally you'd remove one).
That takes us back up to viardh's concluding remark from earlier in the thread:
> You'll of course, as always, find people that takes this way too far. But the general principle is pretty much just to consider where it makes sense to hide linked objects behind a special purpose interface vs. exposing them to clients.
I would say that if you're using a ViewModel object that will be data-bound, then you're sort of outside the realm of the Law of Demeter. It's really more meant to concern objects that implement business logic, not ones that are meant to be fairly dumb data containers.
On the other hand, if it is one that is allowed to implement business logic, then I'd say, yeah, making the children member public in the first place is violating the law. You want to keep that hidden and supply a remove_child() method instead, so that you can retain the ability to change the rules around when and how children are removed, without violating LSP in the process.
3 replies →
FWIW, I was writing JavaScript in that example, so `entity.parent` might have been implemented internally as a function anyway:
I don’t think whether we write `entity.parent` or `entity.parent()` really matters to the argument, though.
In any case, I see what you’re getting at. Perhaps a better way of expressing the distinction I was trying to make is whether the nested object that is being accessed in a chain can be freely used without violating any invariants of the immediate object. If not, as in your example where removing a child has additional consequences, it is probably unwise to expose it directly through the immediate object’s interface.
Yes, its a great case for making the actual `children` collection private so that mutation must go through the interface methods instead. But still, iteration over the children is a likely use case, so you are left with either exposing the original object or returning a copy of the array (potentially slower, though this might not matter depending).
2 replies →