views:

629

answers:

3

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.

A: 

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;
}
Thomas Levesque
I lied just a little bit..it is not really a Singleton :) But I still don't like that the Slave has to be aware of the master even if the master would have been a singleton. Thanks for the reply nonetheless.
Futurist
+6  A: 

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.

Lasse V. Karlsen
Neat! I really like the approach you proposed of the MasterClass taking care of the event hooking, but keeping the events mechanism :) The problem I see is as you asked, there is no guarantee that the Slave will live as long as the Master...so if I want to unhook the event in the Slave destructor, I will bring back the coupling :/
Futurist
Well, there's an alternative, let me change my answer.
Lasse V. Karlsen
+1 Nice answer!
gradbot
Thanks for the indepth reply. I will ponder over this as there is some concepts (like weakreference) that are still unfamiliar to me.
Futurist
I was wondering something. If I keep my current method with the Slave collection, would a good way to force garbage collect the Slaves if to do something like (foreach var slave...if(slave == null) list.Remove)? or that doesn't make much sens I guess.
Futurist
A: 

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.

spender
I lied just a little bit..it is not really a Singleton :) But I still don't like that the Slave has to be aware of the master even if the master would have been a singleton. Thanks for the reply nonetheless.
Futurist