patterncsharpMinor
Message bus in C#
Viewed 0 times
messagebusstackoverflow
Problem
I wrote a
I encountered a problem with duplicate code caused by requirement for same methods with and without generic types, however it does bother me and a better solution would be appreciated.
```
using System;
using System.Collections.Generic;
using System.Linq;
///
/// Class to deliver messages from one object to another without directly linking them.
/// It pushes messages to channels on which objects can subscribe.
///
public static class MessageBus
{
///
/// Dictionary of message listeners
///
private static Dictionary Listeners { get; } = new Dictionary();
///
/// Send message to the bus
///
/// Channel key
/// Arguments of message
public static void Send(object key, T arguments)
{
var fullKey = new ChannelKey(key, typeof(T));
if (!Listeners.ContainsKey(fullKey))
{
return;
}
var actionList = (List>)Listeners[fullKey];
foreach (var listener in actionList)
{
listener.Reaction(arguments);
}
}
///
/// Send message to the bus
///
/// Channel key
public static void Send(object key)
{
var fullKey = new ChannelKey(key);
if (!Listeners.ContainsKey(fullKey))
{
return;
}
var actionList = (List)Listeners[fullKey];
foreach (var listener in actionList)
{
listener.Reaction();
}
}
///
/// Subscribe on messages from channel with given key and given argument type
///
/// Subscriber object
/// Channel key
/// Action to react on message
public static void Subscribe(this object subscriber, object key, Action listenerAction)
{
var fullKey = new ChannelKey(key, typeof(T));
if (!Listeners.ContainsKey(fullKey))
{
Listeners[fullKey] = ne
MessageBus class whose only purpose is to deliver messages from object to object without direct link between them.I encountered a problem with duplicate code caused by requirement for same methods with and without generic types, however it does bother me and a better solution would be appreciated.
```
using System;
using System.Collections.Generic;
using System.Linq;
///
/// Class to deliver messages from one object to another without directly linking them.
/// It pushes messages to channels on which objects can subscribe.
///
public static class MessageBus
{
///
/// Dictionary of message listeners
///
private static Dictionary Listeners { get; } = new Dictionary();
///
/// Send message to the bus
///
/// Channel key
/// Arguments of message
public static void Send(object key, T arguments)
{
var fullKey = new ChannelKey(key, typeof(T));
if (!Listeners.ContainsKey(fullKey))
{
return;
}
var actionList = (List>)Listeners[fullKey];
foreach (var listener in actionList)
{
listener.Reaction(arguments);
}
}
///
/// Send message to the bus
///
/// Channel key
public static void Send(object key)
{
var fullKey = new ChannelKey(key);
if (!Listeners.ContainsKey(fullKey))
{
return;
}
var actionList = (List)Listeners[fullKey];
foreach (var listener in actionList)
{
listener.Reaction();
}
}
///
/// Subscribe on messages from channel with given key and given argument type
///
/// Subscriber object
/// Channel key
/// Action to react on message
public static void Subscribe(this object subscriber, object key, Action listenerAction)
{
var fullKey = new ChannelKey(key, typeof(T));
if (!Listeners.ContainsKey(fullKey))
{
Listeners[fullKey] = ne
Solution
Using
Internally these three methods are calling the
Like you can see I have added braces for the
If you are bothered by duplicated code due to the usage of the generic/non generic version you should maybe think about removing the non generic version. Without seeing the calling code and all the possibilities how the non generic version and the generic version is used its hard to tell if removing the code duplication is possible.
In general your code looks good. You are using well named methods (except for
I wouldn't have the
I don't quite see why the
ContainsKey() together with the getter of the Item property of a Dictionary should be replaced by a call to TryGetValue() which is faster because the check if the key exists is done by the Item getter too. Internally these three methods are calling the
FindEntry() method to check whether a given key exists. So calling this method only once through the TryGetValue() method should be the way to go for instance like so public static void Send(object key, T arguments)
{
var fullKey = new ChannelKey(key, typeof(T));
object value;
if (!Listeners.TryGetValue(fullKey, out value))
{
return;
}
var actionList = (List>)value;
foreach (var listener in actionList)
{
listener.Reaction(arguments);
}Like you can see I have added braces for the
if statement as well. Although they might be optional you should add them always to make your code less error prone. If you are bothered by duplicated code due to the usage of the generic/non generic version you should maybe think about removing the non generic version. Without seeing the calling code and all the possibilities how the non generic version and the generic version is used its hard to tell if removing the code duplication is possible.
In general your code looks good. You are using well named methods (except for
Reaction IMO) and the documentation makes it easy to understand what the code is doing. I wouldn't have the
Listeners as a private property but that's a matter of taste. I personally would prefer just a private static readonly field. I don't quite see why the
MessageReaction should know to which Subscriber it belongs. If the MessageReaction would want to talk to its "parent" it should use events so there is no need to couple this objects that much.Code Snippets
public static void Send<T>(object key, T arguments)
{
var fullKey = new ChannelKey(key, typeof(T));
object value;
if (!Listeners.TryGetValue(fullKey, out value))
{
return;
}
var actionList = (List<MessageReaction<T>>)value;
foreach (var listener in actionList)
{
listener.Reaction(arguments);
}Context
StackExchange Code Review Q#119192, answer score: 7
Revisions (0)
No revisions yet.