Action<Object> callbacks using the mediator method

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 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

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(object sender, TEventArgs e) where TEventArgs : EventArgs;

That's very similar to the Action 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 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.

StackExchange Code Review Q#159149, answer score: 4