patterncsharpMinor
Messenger supporting notifications and requests
Viewed 0 times
notificationssupportingrequestsandmessenger
Problem
I've written a lightweight (I think) class that acts as a messenger service between classes for both notifications (fire and forget updates to other classes) and requests (a notification sent out that expects a returned value).
I'm looking for a general review here on style, usability, best practices, etc.
Here's the code (.NET Fiddle with usage):
```
public class Messenger
{
///
/// Gets the default messenger.
///
/// The default messenger.
public static Messenger Default
{
get
{
defaultMessenger = defaultMessenger ?? new Messenger();
return defaultMessenger;
}
}
///
/// The default messenger
///
private static Messenger defaultMessenger;
///
/// The actions hooked up to messages
///
private Dictionary actions = new Dictionary();
///
/// The functions hooked up to requests
///
private Dictionary functions = new Dictionary();
///
/// Register a function for a request message.
///
/// Type of message to receive.
/// Type of the r.
/// The function that fills the request.
public void Register(Func request)
{
if (request == null)
{
throw new ArgumentNullException("request");
}
if (functions.ContainsKey(typeof(T)))
{
functions[typeof(T)] = Delegate.Combine(functions[typeof(T)], request);
}
else
{
functions.Add(typeof(T), request);
}
}
///
/// Register an action for a message.
///
/// Type of message to receive.
/// The action that happens when the message is received.
public void Register(Action action)
{
if (action == null)
{
throw new ArgumentNullException("action");
}
if (actions.ContainsKey(typeof(T)))
{
actions[typeof(T)] = (Action)Delegate.Combine(actions[typeof(T)], action);
I'm looking for a general review here on style, usability, best practices, etc.
Here's the code (.NET Fiddle with usage):
```
public class Messenger
{
///
/// Gets the default messenger.
///
/// The default messenger.
public static Messenger Default
{
get
{
defaultMessenger = defaultMessenger ?? new Messenger();
return defaultMessenger;
}
}
///
/// The default messenger
///
private static Messenger defaultMessenger;
///
/// The actions hooked up to messages
///
private Dictionary actions = new Dictionary();
///
/// The functions hooked up to requests
///
private Dictionary functions = new Dictionary();
///
/// Register a function for a request message.
///
/// Type of message to receive.
/// Type of the r.
/// The function that fills the request.
public void Register(Func request)
{
if (request == null)
{
throw new ArgumentNullException("request");
}
if (functions.ContainsKey(typeof(T)))
{
functions[typeof(T)] = Delegate.Combine(functions[typeof(T)], request);
}
else
{
functions.Add(typeof(T), request);
}
}
///
/// Register an action for a message.
///
/// Type of message to receive.
/// The action that happens when the message is received.
public void Register(Action action)
{
if (action == null)
{
throw new ArgumentNullException("action");
}
if (actions.ContainsKey(typeof(T)))
{
actions[typeof(T)] = (Action)Delegate.Combine(actions[typeof(T)], action);
Solution
-
Given that
-
I would get rid of the
-
The cast here:
is unnecessary since
-
I'm somewhat dubious about the usefulness being able to register multiple
Given that
Messenger is a fairly light weight class I don't quite see the point of lazy initialization for the static instance. Why not simply use a static initializer and a readonly fieldpublic class Messenger
{
public static readonly Messenger Default = new Messenger();
}-
I would get rid of the
Default static instance. It tends to encourage bad practices. It's easy enough to add later when there is a use case where it really can't be solved any other way but having it there just for convenience purposes is a bad idea.-
The cast here:
actions[typeof(T)] = (Action)Delegate.Combine(actions[typeof(T)], action);is unnecessary since
actions uses Delegate as value type. -
I'm somewhat dubious about the usefulness being able to register multiple
Funcs in an invocation list. The result of calling that invocation list will be the result of the last method being called hence the result will depend on the order of registration which is generally brittle to rely on.Code Snippets
public class Messenger
{
public static readonly Messenger Default = new Messenger();
}actions[typeof(T)] = (Action<T>)Delegate.Combine(actions[typeof(T)], action);Context
StackExchange Code Review Q#71015, answer score: 4
Revisions (0)
No revisions yet.