2

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 sleeped, 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 onBodySleeped() {}
  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?

  • 1
    Short answer to your short question: no. There are no absolutes in software development, and coupling isn't always a bad thing. There has to be some coupling, otherwise you wouldn't be able to transmit data from one object to another. – Robert Harvey May 28 at 18:44
  • @RobertHarvey Yeah, sorry about the lack of conciseness... :) Well, I do agree with you, but some people have been trying to convince me otherwise. Gathering more opinions is always nice to gain some perspective. – Jeto May 28 at 18:47
4

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

  • Thanks a lot for your input (upvoted). Obviously my example here is a bit contrived. In my actual project, children can be quite complex and affect their parent in many ways, and it'd be a mess to try and calculate all possible changes to the parent's attributes from its own perspective. So the parent does publish an interface that these "effects" children can play with, which works pretty well. Does the alternative I suggest in the OP (passing itself as argument to its children instead of having the children hold a reference to their parent) sound better to you, in that regard? – Jeto May 28 at 19:07
  • no the alternative you suggest essentially does the same as injecting in this respect. However, consider that for all the operations a child might perform on the parent, the parent has had to provide the public interface for them to do that operation. It's just as easy for the child to provide the interface for the parent. ie Illness.focusModifer vs Body.set_Focus – Ewan May 28 at 19:11
  • I see what you mean, but I feel like in actual cases it could also make the code a bit counter-intuitive / messy. Say a specific illness, when contracted, must increase the stamina only if the body has a strength > 5, then as soon as it sleeps, start increasing focus by +1 every hour or something like that (everything properly stored in constants, obviously). But then other illnesses have completely different/distinct rules. All of it may be doable in a "topdown"-only way within Body's class, but could make things quite awkward to maintain, don't you think? – Jeto May 28 at 19:30
  • I do like the idea of having a public precedence (or priority) property/method within the child class though, which should cover the case you mention in your answer. Whichever object applies the illnesses would have to do it in proper order (in the "alternative" version from the question, the body would be that object, so it would control that at least). – Jeto May 28 at 19:34
  • so the problem with being conditional on some, non generic to illness, property of body is you instantly run into conflicts with the other possible modifiers, so things like the order of calculation, or who gets event X first can drastically affect the result, or even cause an infinite loop and crash. You can go some way down that path to make things neater, say passing in a IBodyStats with unmodifable properties to your illness.GetModifier. But if you let the child do their part of the calc wiothout restriction you will get bugs *(#unforseen consequences of specifications) – Ewan May 28 at 19:46
4

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.

  • Very smart/simple idea. No matter which path I go for, exposing "limiting interfaces" to children has, indeed, the double advantage of eliminating the coupling and preventing those same children from doing things they're not supposed to. I wish I could accept both answers as I'll probably go for an in-between approach for now. Thank you very much. – Jeto May 28 at 20:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.