Storing methods with varying parameters into a dictionary in Mediator design pattern

Hey all, can anyone lend me a hand on this or inform me if there's a better way? Insofar, I have a Mediator where subjects subscribe themselves. The Mediator sends messages to the target subject from another subject and these messages ought to invoke events/methods of that target subject.

My problem right now is within the subject. They have a Dictionary<string, Method> (not sure what method should be to solve this problem, I've tried from creating my own delegate to using System.Action and UnityEvent). The subject adds to and removes methods from the dictionary. When a subject receives a message, they will invoke said method from their dictionary and do it.
The problem is the child that inherits Subject has to follow the SubjectAction(params object[] args), so I'm forced to make methods like void DoSomething(object[] args) { }.
My team is planning to scale so this is an unsightly solution, especially when developers are forced to do this when they want to subscribe to the Mediator:

Code (CSharp):

1. //GameManager script inheriting from Subject
2. void PlayerBuzzIn(object[] args)
3.     {
4.         if ((int)args[0] == 1)
5.         {
6.             teamOneLock = true;
7.             buttonLock = true;
8.         }
9.         else if((int)args[0] == 2)
10.         {
11.             teamTwoLock = true;
12.             buttonLock = true;
13.         }
14.     }
15.  
16.     void PlayerBuzzReset(object[] args)
17.     {
18.         buttonLock = false;
19.     }
20.  
21.     void AddTeamOneScore(object[] args)
22.     {
23.         teamOneScore += (int)args[0];
24.         Debug.LogFormat("Team One got {0} points!", RoundPoints);
25.     }

I want to make so it the parameters are more along the lines of void AddTeamOneScore(int I) rather than having to check through the array. Even more so a method like void PlayerBuzzIn(bool buzzed in, int player).
Is there a better way of going about this?
I'm not so well versed with delegates and this is my first time implementing a Mediator design, so any help, feedback and criticism is greatly appreciated. Thanks!

Code (CSharp):

1. public abstract class Subject : MonoBehaviour, I_Subject
2. {
3.     public delegate void SubjectAction(params object[] args);
4.     public event SubjectAction SubjectEvent;
5.  
6.     private A_Mediator mediator;
7.     public A_Mediator Mediator
8.     {
9.         get { return mediator; }
10.         set { mediator = value; }
11.     }
12.  
13.     /// <summary>
14.     /// Dictionary to store all of this subject's events.
15.     /// </summary>
16.     private Dictionary<string, SubjectAction> eventDictionary;
17.  
18.     /// <summary>
19.     /// Initializes a dictionary if there wasn't one created yet.
20.     /// </summary>
21.     protected void InitEventDictionary()
22.     {
23.         if (eventDictionary == null)
24.         {
25.             eventDictionary = new Dictionary<string, SubjectAction>();
26.         }
27.     }
28.  
29.     #region Events
30.     /// <summary>
31.     /// Triggers an event if found in eventDictionary of the subject's.
32.     /// </summary>
33.     /// <param name="_eventName"></param>
34.     protected void TriggerEvent(string _eventName, params object[] args)
35.     {
36.         if (eventDictionary.TryGetValue(_eventName, out SubjectEvent))
37.         {
38.             SubjectEvent.Invoke(args);
39.         }
40.         else
41.         {
42.             Debug.LogErrorFormat("{0} could not be executed because it was not found in {1}'s dictionary. Are you calling the correct subject or method?", _eventName, this.GetType().ToString());
43.         }
44.     }
45.  
46.  
47.     /// <summary>
48.     /// Adds an event to the subject's dictionary.
49.     /// </summary>
50.     /// <param name="_eventName"></param>
51.     /// <param name="_listener"></param>
52.     protected void AddEvent(string _eventName, SubjectAction _listener, params object[] args)
53.     {
54.         if (eventDictionary.TryGetValue(_eventName, out SubjectEvent))
55.         {
56.             SubjectEvent += _listener;
57.         }
58.         else
59.         {
60.             SubjectEvent += _listener;
61.             eventDictionary.Add(_eventName, SubjectEvent);
62.         }
63.     }
64.  
65.     /// <summary>
66.     /// Removes an event from the subject's dictionary.
67.     /// </summary>
68.     /// <param name="_eventName"></param>
69.     /// <param name="_listener"></param>
70.     protected void RemoveEvent(string _eventName, SubjectAction _listener)
71.     {
72.         //UnityEvent myEvent = null;
73.         if (eventDictionary.TryGetValue(_eventName, out SubjectEvent))
74.         {
75.             SubjectEvent -= _listener;
76.         }
77.     }
78.     #endregion
79. }

 

Just for a little clarification. The primary purpose of a Mediator pattern is to decouple one "component" class from all others, significantly reducing coupling and communication traffic. Its so class A can connected with class B without knowing nor caring about the existence of a class C. This allows classes to remain self-contained and easy to manage.

Any programmer in Unity should be familiar with this pattern because this is exactly how GameObjects "mediate" its components (hence that "component" hint I mentioned earlier). A Gameobject can have a Renderer, box collider, and a Rigidbody, but the Meditation Pattern allows that Rigidbody to communicate with the Collider without actually messing up the renderer. Want to grab that AudioSouce component in code? well you get it via GetComponent, in other words you are asking the mediator (the gameobject) to get the component for you.

with that big example laid in front of you hopefully that gives you a hint at what you should likely do. if class A wants to communicate with class B it will need to call mediator.GetComponent<ClassB>(). Don't fret over an overly generic method that each of your component classes need to implement, reusing one method to do all sorts of things can easily confuse code intent. Instead, have it so that you can try to get an abstract "component" by interface and use that interface to define the functions you intend to call (exactly like how GameObject.GetComponent<T>() works with interfaces).

Take advantage of the familiar Unity API, and write your classes in a similar fashion. In doing so it'll quickly become second nature in how you want your system to expand and function. And new users will appreciate the familiar API and pickup on it rather quickly. I'm not saying copy it verbatim, you're not planning to replace the purpose of a GameObject or its components, but learn from the design decisions the Unity team went through when they cooked up their mediator pattern (especially the fake null, try not to repeat that).

 

If I'm understanding correctly and my apologies if I'm just regurgitating what you said, have the abstract subject class only to subscribe itself to Mediator and not worry about anything else, Mediator has a method that these subjects can call similarly to GetComponent<T> in order to call the target subject' s respective methods.

That does make a lot more sense. Thank you for the hint! I will update on my progress.
I will also look up some of the process Unity has gone about creating their system, the best practices are essentially the tools we use. Thanks for the heads up about that too.

 

Hey, so this is what I've got so far editing the Mediator to implement what was suggested:

Code (CSharp):

1.    
2. private Dictionary<string, Subject> subjects = new Dictionary<string, Subject>();
3.  
4. /// <summary>
5.     /// Send a string message to a subject to receive and decipher.
6.     /// </summary>
7.     public override T Call<T>()
8.     {
9.         T gameObject = new Component() as T;
10.         Debug.Log(gameObject);
11.         return subjects[gameObject.GetType().Name].GetComponent<T>();
12.     }

And I'm able to do something like Mediator.Call<GameManager>().Progress();
The class Subject adheres to the Mediator and nothing else.
The issue now is that the T gameObject comes back as null. I'm definitely writing this wrong, any help please?

 

What does Subject derive from? UnityEngine.Object? Its own abstract class? And how are you adding in subjects and how are you removing them? Also Component is already a used class name in UnityEngine so you should either change the name or use namespaces (rather you should always use namespaces it'll help later especially if you include 3rd party assets and its just a good habit to start on).

Code (CSharp):

1. private List<Subject> subjects = new List<Subject>();
2.  
3. public virtual T GetSubject<T>()
4. {
5.     Type type = typeof(T);
6.  
7.     for(int i=0;i<subjects.Count;i++)
8.     {
9.         var subject = subjects[i];
10.  
11.         if(subject == null) continue;
12.  
13.         var subjectType = subject.GetType();
14.  
15.         if(subjectType.Is<T>())
16.             return subject as T;    
17.     }
18.     return null;
19. }
20.  
21. //...
22. //from my public static TypeExtensionsClass
23. public static bool Is<T>(this Type type)
24. {
25.     if(type == null) return false;
26.     Type baseType = typeof(T);
27.  
28.     return baseType.IsAssignableFrom(type);
29. }

whatever is calling Mediator.GetSubject<GameManager>() should cache that into a variable and check if its null.
you should do that because thats what you **SHOULD** be doing with GetComponent. Write your code to always first assume that the result might be null/invalid.

Code (CSharp):

1. var manager = mediator.GetSubject<GameManager>();
2.  
3. if (manager != null)
4.     manager.Progress();

 

Subject is it's own abstract class:

Code (CSharp):

1. public abstract class Subject : MonoBehaviour
2. {
3.     private A_Mediator mediator;
4.     public A_Mediator Mediator
5.     {
6.         get { return mediator; }
7.         set { mediator = value; }
8.     }
9. }

Any GameObjects that wants to communicate with the Mediator would inherit Subject. There will only be one instance of such GameObject too.

Code (CSharp):

1.     private Dictionary<string, Subject> subjects = new Dictionary<string, Subject>();
2.  
3.     /// <summary>
4.     /// Adds a subject to library for Mediator to sort through and dictate communication.
5.     /// </summary>
6.     public override void Register<T>(T _subject)
7.     {
8.         string key = _subject.GetType().ToString();
9.  
10.         if (_subject is Subject)
11.         {
12.             Subject tmpSub = _subject as Subject;
13.             if (!subjects.ContainsValue(tmpSub))
14.             {
15.                 subjects[key] = tmpSub;
16.             }
17.             subjects[key].Mediator = this;
18.         }
19.     }
20.  
21.     /// <summary>
22.     /// Deletes a subject from library for Mediator to no longer communicate.
23.     /// </summary>
24.     public override void Delete<T>(T _subject)
25.     {
26.         if (_subject is Subject)
27.         {
28.             Subject tmpSub = _subject as Subject;
29.             if (subjects.ContainsValue(tmpSub))
30.             {
31.                 subjects.Remove(tmpSub.GetType().Name);
32.             }
33.         }
34.     }
35.  
36.     /// <summary>
37.     /// Allows call to type T components.
38.     /// </summary>
39.     public override T Call<T>()
40.     {
41.         T myType = new T();
42.         if (subjects.ContainsKey(myType.GetType().Name))
43.         {
44.             return subjects[myType.GetType().Name].GetComponent<T>();
45.         }
46.         Debug.LogErrorFormat("You are calling {0} but it is probably not in the registry. Check that {0} registered itself to Mediator instance.", myType.GetType().Name);
47.         return default(T);
48.     }

I've just got it too. It's giving me a warning about trying to creating a Monobehavior because of my use of "new".


UPDATE:
Change T myType = new T(): to
Type myType = typeof(T);

That helped rid the warning. Everything works perfectly. Thanks a lot Joshua, and yes, I'll keep in mind about code to check for null. I usually do that in my practices but I was going headstrong on this particular code since my deadline is this Friday and I have a bunch of other things to navigate through. But this is a big win so I really appreciate the time you've spent to help me out!

 

I was suspicious. when you started using variable names like "T gameObject" and "new Component".

So Subject derives from Monobehaviour, this makes (made: seems you fixed some) parts of your code wrong. You can't use the new() constructor on most UnityEngine Objects, including Component/Monobehaviour. and in Call you shouldn't be creating a new instance. nor passing back a default.

If you can't find it. pass back null not default! null is important as its a definitive response from the mediator that lets the caller know that what they are asking for is not there.

using a Dictionary keyed to a type is also a limiting factor. it'd break abstraction(you'd only store concrete types), polymorphism(you can't generically grab a type), and composition(you couldn't check if the mediator is composed of a type). Say for example you add a Button to the meditator. Well it'll get added as "Button" as the key. But button derives from UISelectible, and several IEventSystemHandler interfaces. but if you tried something like meditator.Call<IPointerClickHandler>() it would not find it, even though button is there and is that type. Worse still if you happen to write your own class and also call it button (in your namespace) adding it to the mediator would clash, smash, and remove the Unity button from the dictionary because they would both occupy the same key.Using Dictionary will work... but I wouldn't really say its the right tool for the job here as it limits the meditators flexibility in how its components are grabbed.

This is why I used a List in my example and simply loop and return the first correct instance where the subject is assignable from the target type doing so allows you to call the button as a UISelectible or as an IPointerClickHandler. If you want to limit the count of a specific type you can check if a specific type exists and exit early in the register call.

I'm not exactly sure what purpose your Mediator has and how its different from GameObject. This code looks like it really is trying to replace what GameObject does for you. At which point your just reinventing the wheel. I'm not sure though. I mean your mediator might be spanning multiple gameobjects adding to a sort of unique form of mediation of components and that could be cool, albeit rather ambiguous for end-users.

 

I could've sworn it wouldn't let me return a null if failed check. I'll try again, but that's why I defaulted T since that was the solution given to me by VS correction until I found something better.

The way I'm using Mediator is kinda a specific case. Yeah, the intention is only have concrete, precise types like class GameManager or class SomeRandomClass subscribe themselves to the Mediator. As in the case of Button, that can be taken care of by creating a UIManager that has those respective button calls, then subscribing UIManager to Mediator, no? And there's only one instance of each type hence using type in dictionary. Essentially the Mediator is a manager of managers and from what I was thinking, aids in the communication between these gameObject managers without having to have a direct reference.

This way of thinking stems from a previous version of the project using 200 lines of GameObject.Find(...) and wanting to decouple as much as possible between each managing type gameObject.

 

Thats because no constraints were provided to the generic function. the compiler is assuming T can be a valuetype which can't be null. just add a "where T:new()" so it knows you can't do something like mediator.Call<float>().

As long as you understand how Mediator is meant to decouple things. Here it seems you want to use it to pass references between singleton mangers. Just know that using the Type name as the key doesn't reduce the coupling, but almost encourages it, so be careful with that. If you're fine with this deviation from the pattern then you should be good to go.

If not a cleaner way to enforce one instance of a type is to make a ContainsType() method that looks for the concrete type in the collection, but still store and reference it in its most abstract Type (i.e. Subject). then during register you get the type of the registering Subject and see if subjects already ContainsType(), if so don't add (and prolly return false to communicate failure). or if your dead-set on still using dictionaries don't use the key when inside the Call<T>() method, manually traverse the dictionary for the intended type. this way you can still use the dictionary to ensure unique types in subjects, but the allow Call<T> to handle any abstract type (and use Dictionary<Type,Subject> instead, better support against name conflicts).

 

Ah, so the constraint issue was before I made the fix to typeof. In the abstract Mediator that this code is inheriting from, the method call does do the where T: new() now so I'll make that change when I get on a computer, haha.

I was actually thinking in changing the dictionary to as you suggested so I'm glad I'm on the right train of thought.

As for what you're suggesting of checking against subjects and their types in the collection, that sounds like a great idea. While I'm not deadset on using dictionary, I find it easier to read and use. But I will look into making this system as flexible as I can.

 

where T : class would probably be a better choice if you're just trying to constrain to reference types. On the off chance you pass in something that doesn't have a parameter-less constructor.