views:

333

answers:

6

I have two classes that each need an instance of each other to function. Ordinarily if an object needs another object to run, I like to pass it in the constructor. But I can't do that in this case, because one object has to be instantiated before the other, and so therefore the second object does not exist to be passed to the first object's constructor.

I can resolve this by passing the first object to the second object's constructor, then calling a setter on the first object to pass the second object to it, but that seems a little clunky, and I'm wondering if there's a better way:

backend = new Backend();
panel = new Panel(backend);
backend.setPanel();

I've never put any study into MVC; I suppose I'm dealing with a model here (the Backend), and a view or a controller (the Panel). Any insights here I can gain from MVC?

+2  A: 

In a circular construction scenario I'd use a factory class/factory method. I would normally make the construction logic private to the factory (using friend construct, package level protection or similar), to en sure that no-one could construct instances without using the factory.

The use of setter/constructor is really a part of the contract between the two classes and the factory, so I'd just use whichever's convenient.

As has been pointed out, you really should try to find a non-circular solution.

krosenvold
+1  A: 

It's better to avoid circular references. I would personally try to rethink my objects.

Loki
This answer is not particularly helpful there are instances where a child needs to know who it's parent is. There are safe solutions (like Proxy) when you need to do this.
RS Conley
Well, it was helpful to me in that it confirmed my suspicion that I needed to rethink my objects. :) I think I'm going to go with an MVC listener/event notification type design as suggested in another answer. Which I think is still circular, but somewhat better.
skiphoppy
+8  A: 

It's time to take a look at MVC. :-) When you have a model-view-controller situation, the consensus is that the model shouldn't be aware of the view-controller (MVC often plays out as M-VC), but the view is invariably aware of the model.

If the model needs to tell the view something, it does so by notifying its listeners, of which it may have multiples. Your view should be one of them.

Paul Brinkley
I would add that the view's "knowledge" of the model should be channelled through a well defined interface/api. It should be possible to replace one model with another that implements the same API, such as for testing.
Paul Tomblin
Indeed. This is one of the main reasons to have an MVC pattern in the first place.
Paul Brinkley
Many thanks, Paul. I've posted a description as to how this is going to make my code a lot better.
skiphoppy
+2  A: 

First of all, contrary to what others has said here, there's no inherent problem with circular references. For example, an Order object would be expected to have a reference to the Customer object of the person who placed the Order. Similarly, it would be natural for the Customer object to have a list of Orders he has placed.

In a refernce-based language (like Java or C#) there's no problem, at all. In a value-based language (like C++), you have to take care in designing them.

That said, you design of:

backend = new Backend();
panel = new Panel(backend);
backend.setPanel(panel);

It pretty much the only way to do it.

James Curran
A: 
panel = new Panel(backend);

You do this in this routine something like

  Public Sub Panel(ByVal BackEnd as BackEnd)
        Me.MyBackEnd = BackEnd
        BackEnd.MyPanel = Me
  End Sub

You don't need BackEnd.SetPanel

It is better to use Proxies. A proxy links one object to another through raising a Event. The parent hands the child a proxy. When the child needs the parent it calls a GetRef method on the proxy. The proxy then raises a event which the parent uses to return itself to the proxy which then hands it to the child.

The use of the Event/Delegate mechanism avoids any circular reference problems.

So you have (assuming that the backend is the 'parent' here)

  Public Sub Panel(ByVal BackEnd as BackEnd)
        Me.MyBackEnd = BackEnd.Proxy
        BackEnd.MyPanel = Me
  End Sub

  Public Property MyBackEnd() as BackEnd
     Set (ByVal Value as BackEnd)
        priBackEndProxy = BackEnd.Proxy
     End Set
     Get
        Return priBackEndProxy.GetRef
     End Get
  End Property

Here is a fuller discussion on the problem of circular references. Although it is focused on fixing it in Visual Basic 6.0.

Dynamic Memory Allocation

Also another solution is aggregating Panel and BackEnd into another object. This is common if both elements are UI Controls and need to behave in a coordinated manner.

Finally as far as MVC goes I recommend using a a Model View Presenter approach instead.

Basically you have your Form Implement a IPanelForm interface. It registers itself with a class called Panel which does all the UI logic. BackEnd should have events that Panel can hook into for when the model changes. Panel handles the event and updates the form through the IPanelForm interface.

  1. User clicks a button

  2. The form passes to Panel that the user clicked a button

  3. Panel handles the button and retrieves the data from the backend

  4. Panel formats the data.

  5. Panel uses IPanelForm Interface to show the data on the Form.

RS Conley
A: 

I've been delaying implementing the lessons learned here, giving me plenty of time to think about the exact right way to do it. As other people said, having a clear separation where the backend objects have listeners for when their properties change is definitely the way to go. Not only will it resolve the specific issue I was asking about in this question, it is going to make a lot of other bad design smells in this code look better. There are actually a lot of different Backend classes (going by the generic class names I used in my example), each with their own corresponding Panel class. And there's even a couple of places where some things can be moved around to separate other pairs of classes into Backend/Panel pairs following the same pattern and reducing a lot of passing junk around as parameters.

The rest of this answer is going to get language specific, as I am using Java.

I've not worried a whole lot about "JavaBeans," but I have found that following basic JavaBean conventions has been very helpful for me in the past: basically, using standard getters and setters for properties. Turns out there's a JavaBean convention I was unaware of which is really going to help here: bound properties. Bound properties are properties available through standard getters and setters which fire PropertyChangeEvents when they change. [I don't know for sure, but the JavaBeans standard may specify that all properties are supposed to be "bound properties." Not relevant to me, at this point. Be aware also that "standard" getters and setters can be very non-standard through the use of BeanInfo classes to define a JavaBean's exact interface, but I never use that, either.] (The main other JavaBean convention that I choose to follow or not as appropriate in each situation is a no-argument constructor; I'm already following it in this project because each of these Backend objects has to be serializable.)

I've found this blog entry, which was very helpful in cluing me into the bound properties/PropertyChangeEvents issue and helping me construct a plan for how I'm going to rework this code.

Right now all of my backend objects inherit from a common class called Model, which provides a couple of things every backend in this system needs including serialization support. I'm going to create an additional class JavaBean as a superclass of Model which will provide the PropertyChangeEvent support that I need, inherited by every Model. I'll update the setters in each Model to fire a PropertyChangeEvent when called. I may also have JavaBean inherited by a couple of classes which aren't technically Models in the same sense as these but which could also benefit from having other classes registered as listeners for them. The JavaBean class may not fully implement the JavaBean spec; as I've said, there are several details I don't care about. But it's good enough for this project. It sounds like I could get all this by inheriting from java.awt.Component, but these aren't components in any sense that I can justify, so I don't want to do that. (I also don't know what overhead it might entail.)

Once every Model is a JavaBean, complete with PropertyChangeEvent support, I'll do a lot of code cleanup: Models that are currently keeping references to Panels will be updated and the Panels will register themselves as listeners. So much cleaner! The Model won't have to know (and shouldn't have known in the first place) what methods the Panel should call on itself when the property updates.

skiphoppy