Tight coupling between parent and children: always to be avoided?
https://softwareengineering.stackexchange.com/questions/392505
-
25-02-2021 - |
Question
Say we consider two inherently coupled elements, using a real-life like example:
Body
PhysicalIllness
Note: the following code is pseudo-Java for the sole purpose of syntax-coloring, pure syntax doesn't really matter here.
Body
is a class that holds several basic properties, such as stamina
, focus
, etc., plus an array of PhysicalIllness
representing all the
illnesses it has contracted.
class Body {
int focus
int stamina
PhysicalIllness[] physicalIllnesses
}
PhysicalIllness
is an abstract class, extended by concrete illnesses. Everything they do/react to depend on their host body. They are born inside a body, they "live" within it and their existence doesn't mean anything outside of a body.
Questions
In such a scenario, wouldn't having a Body
instance injected into PhysicalIllness
's constructor, and stored as a (say, host_body
) reference used throughout the life of the illness, be fine? The illness could then respond to life events (say slept
, hour_elapsed
) on its own and impact its host body accordingly:
abstract class PhysicalIllness {
Body hostBody
PhysicalIllness(Body hostBody) {
this.hostBody = hostBody
}
void onAcquired() {}
void onHourElapsed() {}
void onBodySlept() {}
void onGone() {}
}
class Headache extends PhysicalIllness {
void onAcquired() {
this.hostBody.focus -= 10
}
void onHourElapsed() {
this.hostBody.focus += 2
}
// ...
}
Tight coupling actually seems natural to me here. However, it does produce a cyclic/circular dependency, as Body
holds references to PhysicalIllness
instances and PhysicalIllness
also holds a reference to its "parent", its host body.
Could you people point to any downside of designing things this way, in terms of code maintenance/unit-testing, flexibility, or anything else? I realize there are other answers about this, but since every scenario is different, I'm still unsure if they apply here as well.
Alternative (without circular dependency)
One alternative would obviously be to remove the coupling by having PhysicalIllness
instances be notified of every event by the body (which would pass itself as argument in the process). This requires every method of PhysicalIllness
to have a Body
parameter:
abstract class Illness {
void onAcquired(Body hostBody) {}
void onHourElapsed(Body hostBody) {}
// ...
}
class Headache extends Illness {
void onAcquired(Body hostBody) {
hostBody.focus -= 10
}
void onHourElapsed(Body hostBody) {
hostBody.focus += 2
}
// ...
}
class Body {
// ...
void onHourElapsed() {
for (PhysicalIllness illness in this.physicalIllnesses) {
illness.onHourElapsed(this);
}
}
// ...
}
I feel like this is clunky and actually less logical, because it means a physical illness can exist outside of a body (you can construct one without a host body), and therefore all methods require the "obvious" host_body
parameter.
If I had to summarize this post with one single question: should tight coupling and/or circular dependency between parent/children components be avoided in all situations?
Solution
Generally I would try to avoid having a child object hold a reference to its parent. On reflection I think I've seen enough bad designs done this way that I might be tempted to call it an anti-pattern.
Sure there are cases where it makes sense, such as navigating a tree structure, but its often a shortcut to information outside the concern of the class. It begs the question, what public interface does Body expose? should you be able to change focus at all?
Using your example, consider the case where a body has two illnesses. A decreases focus by 2 and B multiples focus by 2. Now the order in which you apply these two modifiers is important. Are you going to have illness A check to see if illness B is present? Tempting; but it would lead to a very messy design.
If instead you have the parent calculate its property as a function of its illnesses your life will be easier. The parent knows about its own events and all its illnesses and the illnesses can expose a common interface which allows the parent to perform the calculation, eg Illness.Precidence or whatever. Furthermore the Parent class can freely modify its own properties and does have to worry about exposing them to outside change via its public interface.
I'm not sure I would label these kind of relationship tight coupling. the classes are tightly coupled due to one referencing the other yes. But what you are doing goes further
OTHER TIPS
Avoidance of bidirectional coupling is not an end in itself, it is a means to an end. Here, it will lead to a situation where it becomes impossible to instantiate a PhysicalIllness
object alone, without creating a Body
object first. That can be an issue, or not, this depends heavily on the case.
I see only a problem if creation of the Body
requires a lot of effort and so, for example, makes reusage and testing of PhysicalIllness
objects harder. But if a Body
object is simple, and does not contain many more members than those shown in your (simplified) example, than this design will probably not cause any issues.
Note that a Body
object will not necessarily require a PhysicalIllness
object, since the array can be empty, so the cyclic dependency does not cause a "hen-or-egg" problem.
If required, there is a different approach for resolving the cyclic dependency than the one suggested in your question: by introducing an interface IBody
, and let the constructor of PhysicalIllness
get an IBody
injected:
PhysicalIllness(IBody hostBody) {
this.hostBody = hostBody
}
IBody
can provide exactly the methods required by a PhysicalIllness
, no less, no more. That way, PhysicalIllness
might be instantiated with a "mock Body", which is also derived from IBody
, but simpler to create than a Body
object.
You don't want any unnecessary coupling.
But consider this: If you implement a class, all the methods there are very tightly coupled, and that is intentional, and it is fine. So if you have two classes that are very strongly related, that you develop together and change together, then tight coupling is absolutely fine.
(In Swift, you can allow classes to be coupled tighter than normal by putting them into the same file, and "fileprivate" members can be accessed from anywhere in the same source file - even in different classes. If you see "fileprivate" you know it is intentionally).