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?

Was it helpful?

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).

Licensed under: CC-BY-SA with attribution
scroll top