Action<Object> callbacks using the mediator method

Question

I have a WPF project which uses XAML and C#. It adheres (as best as I know) to the MVVM methodology.

As part of it, I have used a mediator class (the 'Gang of Four' design). You register for callbacks, and other classes can notify registered classes in the case of certain events. For example, some parts of the program need to know when the data has been loaded from the database, so they can register to the database and receive a callback when the loading is done.

The 'Register' part of the mediator looks like this:

public void Register(Action<Object> callback, ViewModelMessages message)
{
    internalList.AddValue(message, callback);
}

Classes can register themselves, like so:

Mediator.Mediator.Instance.Register(
            (Object o) =>
            {

                if (o == null)
                    return;
                // Perform action using 'o'.

            }, Mediator.Mediator.ViewModelMessages.SelectedCharacterChanged);

This all works well and good, and my question isn't exactly one of right or wrong, but one of a more elegant design.

One of my callbacks needs to send a boolean value along with it. But Action(object) (I can't use the angular brackets here) has a problem with that when it comes to the callback. O (in the second piece of code) needs to be nullable. If I were to do something like this:

 Mediator.Mediator.Instance.Register(
            (Object o) =>
            {
                // o is boolean
                if (o == null)
                    return;

                if( (o as bool) == true)
                   // Do action


            }, Mediator.Mediator.ViewModelMessages.AddNewCharacterMode);

The program will tell me a bool is not nullable (indeed it is not). I will assume this is going to be a problem for all non nullable types like int and what not.

There are some solutions, but none of them elegant or simple.

The first would be to create a class that, for this instance, just holds a bool.

The second is to alter the Action(object) to be Action(bool). Well this works in this instance, but I use this method for other things, most of which are objects. So then I'd have to create a second set of methods in the mediator to handle the bool, which seems not very good.

The third, which leads on from the second, is to create a generic mediator, but this also isn't good because I'd need a new instance of the mediator for each type. Again it would work, but I think it's going to make things confusing.

If anyone has an elegant solution, I would be appreciative. I feel it can't be a rare problem, so I'm hoping that there's some established way of doing this.

Below is the code for the mediator class:

public sealed class Mediator
{
    public enum ViewModelMessages { SearchPaneViewModel = 1, CharacterFormViewModel = 2, SelectedCharacterChanged = 3, AddNewCharacterMode = 4 };

    #region Data
    static readonly Mediator instance = new Mediator();
    private volatile object locker = new object();

    MultiDictionary<ViewModelMessages, Action<Object>> internalList = new MultiDictionary<ViewModelMessages, Action<Object>>();
    #endregion

    #region Ctor
    //CTORs
    static Mediator()
    {


    }

    private Mediator()
    {

    }
    #endregion

    #region Public Properties

    /// <summary>
    /// The singleton instance
    /// </summary>
    public static Mediator Instance
    {
        get
        {
            return instance;
        }
    }

    #endregion

    #region Public Methods
    /// <summary>
    /// Registers a Colleague to a specific message
    /// </summary>
    /// <param name="callback">The callback to use
    /// when the message it seen</param>
    /// <param name="message">The message to
    /// register to</param>
    public void Register(Action<Object> callback, ViewModelMessages message)
    {
        internalList.AddValue(message, callback);
    }


    /// <summary>
    /// Notify all colleagues that are registed to the
    /// specific message
    /// </summary>
    /// <param name="message">The message for the notify by</param>
    /// <param name="args">The arguments for the message</param>
    public void NotifyColleagues(ViewModelMessages message, object args)
    {
        if (internalList.ContainsKey(message))
        {
            //forward the message to all listeners
            foreach (Action<object> callback in internalList[message])
                    callback(args);
        }
    }
    #endregion

}

Answer 1

Events

I feel it can't be a rare problem, so I'm hoping that there's some established way of doing this.

Indeed it's a very common problem. So common, the people that put the .net framework together decided to solve it. Their implementation is embodied in multicast delegates.

The most common use of multicast delegates, is events. So you have a delegate:

public delegate void EventHandler<TEventArgs>(object sender, TEventArgs e) where TEventArgs : EventArgs;

That's very similar to the Action<object> delegate you're using, with a notable difference: the data you want to carry around is encapsulated in a System.EventArgs object.

public class MyEventArgs : EventArgs
{
    public bool SomeReadWriteState { get; set; }
}

The event source simply needs to expose an event, that the clients can register:

public event EventHandler<MyEventArgs> SomeEvent;

public void DoSomething()
{
    // do stuff
    var args = new MyEventArgs();
    SomeEvent?.Invoke(this, args);

    if (args.SomeReadWriteState)
    {
        // do stuff
    }
}

Of course passing state to multiple clients is problematic - you can't control the order of execution of a multicast delegate's subscribers, and since every client gets the same args, there's something iffy with passing mutable data back to the caller from multiple clients. But typically when you do that, you'd only register a single client anyway.

C# has a handy syntax for registering/unregistering these:

var source = new MyEventSource();
source.SomeEvent += source_SomeEvent;
// source.SomeEvent -= source_SomeEvent;

Where source_SomeEvent is any accessible method that matches the delegate's signature:

private void source_SomeEvent(object sender, MyEventArgs e)
{
    // do stuff
}

And that concludes my events 101 tutorial. You can find more information just about anywhere on Stack Overflow.

---

Review

I don't like this:

public enum ViewModelMessages { SearchPaneViewModel = 1, CharacterFormViewModel = 2, SelectedCharacterChanged = 3, AddNewCharacterMode = 4 };

Declaring enum members on a single line like this makes it much harder to read and extend. Compare to:

public enum ViewModelMessages 
{
    SearchPaneViewModel = 1, 
    CharacterFormViewModel = 2, 
    SelectedCharacterChanged = 3, 
    AddNewCharacterMode = 4,
};

Much easier to see everything at once!

This scares me:

private volatile object locker = new object();

I'm 99.9999% sure you don't need this to be volatile. I don't think I ever needed anything to be volatile, to be honest. But since nothing in the class is using it, the field could be simply removed - I presume it's a leftover from some previous iteration on the Singleton implementation.

static readonly Mediator instance = new Mediator();

Why is the access modifier left implicit? static isn't an access modifier and doesn't replace one. That declaration is implicitly private. Every single other modifier is explicit. Why not this one?

private static readonly Mediator instance = new Mediator();

Why enclose them in a #region? Isn't it obvious that they're constructors? That the private fields are, well, fields? That the public members are public members? This answer on SoftwareEngineering.SE covers #region pretty well. In a nutshell: #region is a relic of the language that predates partial classes; it was used to hide/collapse lengthy generated code that you never need to read or modify in any way. I encourage you to read that answer in whole.

Your Singleton implementation is quite literally the fourth version that Jon Skeet discusses here; I encourage you to read about its pros and cons, that article is extremely complete. That said, Singleton is rarely something you need in object-oriented code.

---

Conclusion

Your pattern is reinventing a wheel that the framework has already crafted for you, and as you noted there's a problem with passing data to and from the callbacks... that the framework solved in a rather elegant and future-proof way, with the generic version of the EventHandler delegate.

Nice for educational/academic purposes, but for production code I'd just use events.