Does the following indicate a bad design?

Question

Hello,

I was wondering whether you think the following code usually indicates a bad design ...

class X
{
public:
  ...
private:
  Y y;
};

Class Y
{
public:
   Y( X& value ){ x = value; };
private:
   X& x;
}

(i.e. there is some sort of cyclic dependency between the classes X and Y).

Answer 1

Depends on what you're trying to do. Some designs, such as iterators, require these kinds of cycles.

Answer 2

No, I don't think it is bad design. It looks like a parent-child relationship (which is 1-to-1) where the child has a pointer back to the parent.

The main reason for such a design is if someone could get a reference to Y without coming through X. If all access to Y are done only through X, such a reference is more questionable.

As Billy ONeal points out, one example could be an iterator that needs to be able to reference the collection it belongs to. This could make it possible to have an iterator into an array that doesn't need to be invalidated when the array is resized, as it would check the array's current buffer address and current size on each access.

Another example could be an OrderLine class (your X) which holds a reference to the item and a quantity count etc. It also has an optional member of type DigitalLicense (your Y). DigitalLicense contains a large encrypted description of the license, so it's only included in those OrderLines that corresponds to a product with a digital license.

On construction, a reference to the DigitalLicense is also put in a separate map, keyed on the license id. It is now possible to look up a DigitalLicense based on the license id in the map. Then the back reference is used to go from the DigitalLicense to the OrderLine. If the OrderLine has a similar back reference it is possible to get back to the Order as well.

Answer 3

I don't think this is problematic. In fact, it's a rather common pattern (i.e. a child object maintains a pointer/reference to parent object).

Answer 4

This is not necessarily bad design, indeed it's fairly common. However, you do need to be very careful with these kind of designs when dealing with memory management. You can end up with tricky bugs when each class depends on the other like this.

Answer 5

in former times e.g. in VB6 this was bad design, because VB couldn't delete both objects, where one was referenced in another and vice versa. since C++ does not have reference counting, it's no problem any more.

Answer 6

Why not just use a better language like say, OCaml or Haskell?!