4
\$\begingroup\$

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

}
\$\endgroup\$
  • \$\begingroup\$ What problem does this pattern solve that multicast delegates (a.k.a. events) don't? The framework's solution to the problem you're describing is to encapsulate the event args in an object derived from the EventArgs base class, so you get EventHandler<TEventArgs> as a generic delegate, with object sender, TEventArgs args as a signature. So... Indeed there's an established way of doing this: events! \$\endgroup\$ – Mathieu Guindon Mar 28 '17 at 18:45
  • \$\begingroup\$ Your terminology is slightly off: you say nullable types to mean reference types. Nullable<T> is a value type that has nullable semantics. \$\endgroup\$ – Mathieu Guindon Mar 28 '17 at 18:53
  • \$\begingroup\$ I guess I shall have to look up events since your answer contains many terms I don't know. Thanks for pointing me in that direction though. \$\endgroup\$ – TheFaithfulLearner Mar 28 '17 at 19:02
  • \$\begingroup\$ You'll have to forgive me. I still am unable to see how this solves my problem. I've read those four lines ten times or more and it feels like all I'm doing is trying to draw the owl. \$\endgroup\$ – TheFaithfulLearner Mar 28 '17 at 19:36
  • \$\begingroup\$ Why don't you simply use the ObservableCollection? \$\endgroup\$ – t3chb0t Mar 28 '17 at 19:40
4
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Thanks for taking the time to help me. I would like to ask - why make the event (SomeEvent) nullable? And is all this supposed to replace the mediator entirely? \$\endgroup\$ – TheFaithfulLearner Mar 29 '17 at 10:37
  • \$\begingroup\$ The type of the event is a delegate type, just like Action<T> is. Delegates are reference types, like classes are, but delegates are not classes; seems you have been using them without realising what they are and what they can do. I never needed to explicitly implement a mediator pattern in .net - events are a mediator pattern. \$\endgroup\$ – Mathieu Guindon Mar 29 '17 at 12:22
  • \$\begingroup\$ Hm, I see. Thanks for the tip and thanks for all the help. \$\endgroup\$ – TheFaithfulLearner Mar 29 '17 at 12:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.