C#:: When to use events or a collection of objects

Question

I have what I think is a simple "problem" to which I have found a couple of solutions but I am not sure which way to go andn the best practice in C#.

I have a master object (say a singleton) instanciated once during the lifespan of the application. This "MasterClass" creates a bunch of new type of objects, say "SlaveClass" every time MasterClass.Instance.CreateSlaveObject is called.

This MasterClass also monitors some other object for status change, and when that happens, notifies the SlaveClass objects it created of the change. Seems simple enough.

Since I come from the native C++ world, the way I did it first it to have an interface

Interface IChangeEventListener
{
    void ChangeHappened();
}

from which I derived "SlaveClass". Then in my "MasterClass" i have:

...
IList<IChangeEventListener> slaveList;
...
CreateSlaveObject
{
    ...
    slaveList.Add(slave);
}
...
ChangeHappened()
{
    ...
    foreach(var slave in slaveList)
    {
       slave.ChangeHappened();
    }
}

And this works. But I kept wondering in the back of my mind if there is another (better) way of doing this. So I researched a bit more on the topic and saw the C# events.

So instead of maintaining a collection of slaves in the MasterClass, I would basically inject the MasterClass into the ctor of SlaveClass (or via a property) and let the SlaveClass object add it's ChangeHappened as an event handler. this would be illustrated:

  ...Master...          
  public delegate void ChangeHappenedDelegate(object sender, NewsInfoArgs args);
  public event NewUpdateDelegate ChangeHappenedEvent;
  ....

  public SlaveClass (MasterClass publisher) //inject publisher service
  {
      publisher.ChangeHappenedEvent += ChangeHappened;
  }

But this seems to be like an un-necessary coupling between the Slave and the Master, but I like the elegance of the provided build-in event notification mechanism.

So should I keep my current code, or move to the event based approach (with publisher injection)? and why?

Or if you can propose an alternative solution I might have missed, I would appreciate that as well.

Answer 1

Well, in my mind, events and interfaces like you showed are two sides of the same coin (at least in the context you described it), but they're really two sides of this.

The way I think about events is that "I need to subscribe to your event because I need you to tell me when something happens to you".

Whereas the interface way is "I need to call a method on you to inform you that something happened to me".

It can sound like the same, but it differs in who is talking, in both cases it is your "masterclass" that is talking, and that makes all the difference.

Note that if your slave classes have a method available that would be suitable for calling when something happened in your master class, you don't need the slave class to contain the code to hook this up, you can just as easily do this in your CreateSlaveClass method:

SlaveClass sc = new SlaveClass();
ChangeHappenedEvent += sc.ChangeHappened;
return sc;

This will basically use the event system, but let the MasterClass code do all the wiring of the events.

Does the SlaveClass objects live as long as the singleton class? If not, then you need to handle the case when they become stale/no longer needed, as in the above case (basically in both of yours and mine), you're holding a reference to those objects in your MasterClass, and thus they will never be eligible for garbage collection, unless you forcibly remove those events or unregisters the interfaces.

---

To handle the problem with the SlaveClass not living as long as the MasterClass, you're going to run into the same coupling problem, as you also noted in the comment.

One way to "handle" (note the quotes) this could be to not really link directly to the correct method on the SlaveClass object, but instead create a wrapper object that internally will call this method. The benefit from this would be that the wrapper object could use a WeakReference object internally, so that once your SlaveClass object is eligible for garbage collection, it might be collected, and then the next time you try to call the right method on it, you would notice this, and thus you would have to clean up.

For instance, like this (and here I'm typing without the benefit of a Visual Studio intellisense and a compiler, please take the meaning of this code, and not the syntax (errors).)

public class WrapperClass
{
    private WeakReference _Slave;

    public WrapperClass(SlaveClass slave)
    {
        _Slave = new WeakReference(slave);
    }

    public WrapperClass.ChangeHappened()
    {
        Object o = _Slave.Target;
        if (o != null)
            ((SlaveClass)o).ChangeHappened();
        else
            MasterClass.ChangeHappenedEvent -= ChangeHappened;
    }
}

In your MasterClass, you would thus do something like this:

SlaveClass sc = new SlaveClass();
WrapperClass wc = new WrapperClass(sc);
ChangeHappenedEvent += wc.ChangeHappened;
return sc;

Once the SlaveClass object is collected, the next call (but not sooner than that) from your MasterClass to the event handlers to inform them of the change, all those wrappers that no longer has an object will be removed.

Answer 2

Although events would be the normal paradigm for exposing public subscribe/unsubscribe functionality, in many cases they're not the best paradigm when subscribe/unsubscribe functionality don't need to be exposed. In this scenario, the only things that are allowed to subscribe/unsubscribe the event are the ones that you yourself create, so there's no need for public subscribe/unsubscribe methods. Further, the master's knowledge of the slaves far exceeds the typical event publisher's knowledge of subscribers. Therefore, I favor having the master explicitly handle the connection/disconnection of subscriptions.

One feature I would probably add, though, which is made somewhat easier by the coupling between master and slave, would be a means of allowing slaves to be garbage-collected if all outside references are abandoned. The easiest way of doing this would probably be to have the master keep a list of 'WeakReference's to slaves. When it's necssary to notify slaves that something has happened, go through the list, derereference any WeakReference that's still alive, and notify the slave. If more than half the items in the list have been added since the last sweep and the list contains at least 250 or so items, copy all live references to a new list and replace the old one.

An alternative approach which would avoid having to dereference the WeakReference objects so often would be to have the public "slave" object be a wrapper to a private object, and have the public object override Finalize to let the private object know that no outside references to it exist. This would require adding an extra level of strong indirection for public accesses to the object, but it would avoid creating--even momentarily--strong references to abandoned objects.

Answer 3

As you appear to have a singleton instance of MasterClass, why not subscribe to MasterClass.Instance.ChangeHappenedEvent? It's still a tight-ish coupling, but relatively neat.

Answer 4

I think it's just a matter of personal preference... personnally I like to use events, because it fits better in .NET "philosophy".

In your case, if the MasterClass is a singleton, you don't need to pass it to the constructor of the SlaveClass, since it can be retrieved using the singleton property (or method) :

public SlaveClass ()
{
    MasterClass.Instance.ChangeHappenedEvent += ChangeHappened;
}