views:

916

answers:

6

How can you avoid circular dependencies when you're designing two classes with a producer/consumer relationship? Here ListenerImpl needs a reference to Broadcaster in order to register/unregister itself, and Broadcaster needs a reference back to the Listeners in order to send messages. This example is in Java but it can apply to any OO language.

public interface Listener {
  void callBack(Object arg);
}
public class ListenerImpl implements Listener {
  public Foo(Broadcaster b) { b.register(this); }
  public void callBack(Object arg) { ... }
  public void shutDown() { b.unregister(this); }
}
public class Broadcaster {
  private final List listeners = new ArrayList();
  public void register(Listener lis) { listeners.add(lis); }
  public void unregister(Listener lis) {listeners.remove(lis); }
  public void broadcast(Object arg) { for (Listener lis : listeners) { lis.callBack(arg); } }
}
+6  A: 

I don't see that being a circular dependency.

Listener depends on nothing.

ListenerImpl depends on Listener and Broadcaster

Broadcaster depends on Listener.

        Listener
       ^        ^
      /          \
     /            \
Broadcaster <--  ListenerImpl

All arrows end at Listener. There's no cycle. So, I think you're OK.

Herms
But there's still a reference cycle - broadcaster has a reference to the concrete ListenerImpl object even though the type of the reference is the Listener interface type. Doesn't a reference cycle imply a dependency cycle?
sk
There isn't really a way to remove a reference cycle. It's kind of required for this type of thing. Reference cycles aren't really a problem these days, given that any good garbage collector should handle them fine. It's dependency cycles that you need to worry about.
Herms
@sk: in order to define 'cycle' you must define 'dependency' first: there is no compile-time cycle. There _is_ a runtime cycle if all references are equal, and somebody needs to break the cycle when it's no longer needed (remove listeners)
xtofl
A: 

I'm not a java dev, but something like this:

public class ListenerImpl implements Listener {
  public Foo() {}
  public void registerWithBroadcaster(Broadcaster b){ b.register(this); isRegistered = true;}
  public void callBack(Object arg) { if (!isRegistered) throw ... else ... }
  public void shutDown() { isRegistered = false; }
}

public class Broadcaster {
  private final List listeners = new ArrayList();
  public void register(Listener lis) { listeners.add(lis); }
  public void unregister(Listener lis) {listeners.remove(lis); }
  public void broadcast(Object arg) { for (Listener lis : listeners) { if (lis.isRegistered) lis.callBack(arg) else unregister(lis); } }
}
Sunny
+5  A: 

Any OOP language? OK. Here's a ten-minute version in CLOS.

Broadcasting framework

(defclass broadcaster ()
  ((listeners :accessor listeners
              :initform '())))

(defgeneric add-listener (broadcaster listener)
  (:documentation "Add a listener (a function taking one argument)
  to a broadcast's list of interested parties"))

(defgeneric remove-listener (broadcaster listener)
  (:documentation "Reverse of add-listener"))

(defgeneric broadcast (broadcaster object)
  (:documentation "Broadcast an object to all registered listeners"))

(defmethod add-listener (broadcaster listener)
  (pushnew listener (listeners broadcaster)))

(defmethod remove-listener (broadcaster listener)
  (let ((listeners (listeners broadcaster)))
    (setf listeners (remove listener listeners))))

(defmethod broadcast (broadcaster object)
  (dolist (listener (listeners broadcaster))
    (funcall listener object)))

Example subclass

(defclass direct-broadcaster (broadcaster)
  ((latest-broadcast :accessor latest-broadcast)
   (latest-broadcast-p :initform nil))
  (:documentation "I broadcast the latest broadcasted object when a new listener is added"))

(defmethod add-listener :after ((broadcaster direct-broadcaster) listener)
  (when (slot-value broadcaster 'latest-broadcast-p)
    (funcall listener (latest-broadcast broadcaster))))

(defmethod broadcast :after ((broadcaster direct-broadcaster) object)
  (setf (slot-value broadcaster 'latest-broadcast-p) t)
  (setf (latest-broadcast broadcaster) object))

Example code

Lisp> (let ((broadcaster (make-instance 'broadcaster)))
        (add-listener broadcaster 
                      #'(lambda (obj) (format t "I got myself a ~A object!~%" obj)))
        (add-listener broadcaster 
                      #'(lambda (obj) (format t "I has object: ~A~%" obj)))
        (broadcast broadcaster 'cheezburger))

I has object: CHEEZBURGER
I got myself a CHEEZBURGER object!

Lisp> (defparameter *direct-broadcaster* (make-instance 'direct-broadcaster))
      (add-listener *direct-broadcaster*
                  #'(lambda (obj) (format t "I got myself a ~A object!~%" obj)))
      (broadcast *direct-broadcaster* 'kitty)

I got myself a KITTY object!

Lisp> (add-listener *direct-broadcaster*
                    #'(lambda (obj) (format t "I has object: ~A~%" obj)))

I has object: KITTY

Unfortunately, Lisp solves most of the design pattern problems (such as yours) by eliminating the need for them.

Mikael Jansson
+3  A: 

In contrast to Herms' answer, I do see a loop. It's not a dependency loop, it's a a reference loop: LI holds the B object, the B object holds (an Array of) LI object(s). They don't free easily, and care needs to be taken to ensure that they free when possible.

One workaround is simply to have the LI object hold a WeakReference to the broadcaster. Theoretically, if the broadcaster has gone away, there's nothing to deregister with anyway, so then your deregistration will simply check if there is a broadcaster to deregister from, and do so if there is.

Tanktalus
References loops aren't really an issue. AFAIK java's garbage collector should handle it just fine. I don't think you'd really gain anything from using a weak reference.
Herms
@Herms: you're getting spoiled by automatic garbage collection :) I find it a _very_ good issue: never simply assume ownership to be allright without giving it a thought! +1
xtofl
A: 

Use weak references to break the cycle.

See this answer.

janm
A: 

Here's an example in Lua (I use my own Oop lib here, see references to 'Object' in the code).

Like in Mikael Jansson's CLOS example, your can use functions directly, removing the need of defining listeners (note the use of '...', it's Lua's varargs):

Broadcaster = Object:subclass()

function Broadcaster:initialize()
    self._listeners = {}
end

function Broadcaster:register(listener)
    self._listeners[listener] = true
end

function Broadcaster:unregister(listener)
    self._listeners[listener] = nil
end
function Broadcaster:broadcast(...)
    for listener in pairs(self._listeners) do
        listener(...)
    end
end

Sticking to your implementation, here's an example that could be written in any dynamic language I guess:

--# Listener
Listener = Object:subclass()
function Listener:callback(arg)
    self:subclassResponsibility()
end

--# ListenerImpl
function ListenerImpl:initialize(broadcaster)
    self._broadcaster = broadcaster
    broadcaster:register(this)
end
function ListenerImpl:callback(arg)
    --# ...
end
function ListenerImpl:shutdown()
    self._broadcaster:unregister(self)
end

--# Broadcaster
function Broadcaster:initialize()
    self._listeners = {}
end
function Broadcaster:register(listener)
    self._listeners[listener] = true
end
function Broadcaster:unregister(listener)
    self._listeners[listener] = nil
end
function Broadcaster:broadcast(arg)
    for listener in pairs(self._listeners) do
        listener:callback(arg)
    end
end
Sébastien RoccaSerra