tags:

views:

211

answers:

7

I encountered this a couple of times now, and i wondered what is the OO way to solve circular references. By that i mean class A has class B as a member, and B in turn has class A as a member.

One example of this would be class Person that has Person spouse as a member.

Person jack = new Person("Jack");
Person jill = new Person("Jill");
jack.setSpouse(jill);
jill.setSpouse(jack);

Another example would be Product classes that have some Collection of other Products as a member. That collection could for example be products that people who are interested in this product might also be interested in, and we want to upkeep that list on a per-product base, not on same shared attributes (e.g. we don't want to just display all other products in the same category).

Product pc = new Product("pc");
Product monitor = new Product("monitor");
Product tv = new Product("tv");
pc.setSeeAlso({monitor, tv});
monitor.setSeeAlso({pc});
tv.setSeeAlso(null);

(these products are just for making a point, the issue is not about wether or not certain products would relate to each other)

Would this be bad design in OOP in general? Would/should all OOP languages allow this, or is it just bad practice? If it's bad practice, what would be the nicest way of solving this?

+2  A: 

The examples you give are (to me, anyway) examples of reasonable OO design.

The cross-referencing issue you describe isn't an artifact of any design process but a real-life characteristic of the things you're representing as objects, so I don't see there's a problem.

What have you encountered that has given you the impression that this approach is bad-design?

Update 11 March:

In systems that lack garbage collection, where memory management is explicitly managed, one common approach is to require all objects to have an owner - some other object responsible for managing the lifetime of that object.

One example is Delphi's TComponent class, which provides cascading support - destroy the parent component, and all owned components are also destroyed.

If you're working on such a system, the kinds of referential loop described in this question may be considered poor design because there's no clear owner, no one object responsible for managing lifetimes.

The way that I've seen this handled in some systems is to retain the references (because they properly capture the business concerns), and to add in an explicit TransactionContext object that owns everything loaded into the business domain from the database. This context object takes care of knowing which objects need to be saved, and cleans everything up when processing is complete.

Bevan
It seems like real-life to me too, i was just thinking won't VMs get confused chasing object... loading a Person, loading that person's spouse, again that person's spouse, ... it's an eternal loop. Just wondering if all VMs can handle that properly.
tehvan
VM? You mean runtimes? Ref-counted only systems (COM or roll-your own smart pointers, for example) will, any real GC system won't
Simon Buchan
Reference cycles are handled just fine by both the CLR (.NET) and JVM environments. They were also handled just fine by the early Lisp and Smalltalk environments, within which much of this technology was originally proved.
Bevan
+1  A: 

The main time this is a problem is if it becomes too confusing to cope with, or maintain, as it can become a form of spaghetti code.

However, to touch on your examples;

See Also is perfectly valid if this is a feature you need in your code - it is a simple list of pointers (or references) to other items a user may be interested in.

Similarily it is perfectly valid to add spouse, as this is a simple real world relationship that would not be confusing to someone maintaining your code.

I have always seen it as a potential code smell, or perhaps a warning to take a step back and rationalise what I am doing.

As for some systems finding recursive relationships in your code (mentioned in a comment above), these can come up regardless of this sort of design. I have recently worked on a metadata capture system that had recursive 'types' of relationships - i.e Columns being logically related to other columns. It needs to be handled by the code trying to parse your system.

johnc
A: 

One way to fix this is to refer to other object via an id.

e.g.

Person jack = new Person(new PersonId("Jack"));
Person jill = new Person(new PersonId("Jill"));
jack.setSpouse(jill.getId());
jill.setSpouse(jack.getId());

I'm not saying it is a perfect solution, but it will prevent circular references. You are using an object instead of a object reference to model the relationship.

parkr
This is thinking too much like a database. Setting a reference to an object accomplishes the same thing with less work.
Boden
But if you want to persist the object graph to a database, this approach will allow partial loading of the object graph without proxying and prevent circular references. As I said, it is not the perfect solution but I have seen it used successfully in a multi-million line of code application.
parkr
+1  A: 

It's not a fundamental problem in OO design. An example of a time it might become a problem is in graph traversal, for instance, finding the shortest path between two objects - you could potentially get into an infinite loop. However, that's something you would have to consider on a case-by-case basis. If you know there could be cross-references in a case like that, then code some checks in to avoid infinite loops (for instance, maintaining a set of visited nodes to avoid re-visiting). But if there's no reason it could be a problem (such as in the examples you gave in your question), then it's not bad at all to have such cross-references. And in many cases, as you've described, it's a good solution to the problem at hand.

Smashery
A: 

I don't think the circular references as such are a problem.

However, putting all those relationships inside objects may add too much clutter, so you may instead want to represent them externally. E.g. you might use a hash table to store relationships between products.

starblue
+1  A: 

I do not think this is an example of cross referencing.

Cross referencing usually pertains to this case:

class A
{
    public void MethodA(B objectB)
    {
       objectB.SomeMethodInB();
    }
}

class B
{
    public void MethodB(A objectA)
    {
        objectA.SomeMethodInA();
    }
}

In this case each object kind of "reaches in" to each other; A calls B, B calls A, and they become tightly coupled. This is made even worse if A and B are in different packages/namespaces/assemblies; in many cases those would create compile time errors as assemblies are compiled linearly.

The way to solve that is to have either object implement an interface with the desired method.

In your case you only have one level of "reaching in":

public Class Person
{
    public void setSpouse(Person person)
    { ... }
}

I do not think this is unreasonable, nor even a case of cross-referencing/circular references.

Jon Limjap
A: 

Referencing other objects is not a real bad OO design at all. It's the way state is managed within each object.

A good rule of thumb is the Law of Demeter. Look at this perfect paper of LoD (Paperboy and the wallet): click here

Patrick Peters