HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Communicating messages to objects

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
messagescommunicatingobjects

Problem

In my MUD game engine, I built a messaging api that allows objects to subscribe to concrete implementations of an IMessage interface. Is there anything glaringly bad about the approach I have taken?

The intended use-case is for objects, such as an IPlayer implementation, to subscribe to WhisperMessage and ShoutMessage publishes. When a player sends a chat message, a mediator performs a Publish for an instance of ShoutMessage and all IPlayer implementations that subscribed are given the message.

IMessage, it's generic counterpart, base class and implementation

```
public interface IMessage
{
///
/// Gets the content.
///
/// Returns the content of the message
object GetContent();
}

///
/// Allows for receiving the content of a message
///
/// The type of the content.
public interface IMessage : IMessage where TContent : class
{
///
/// Gets the content of the message.
///
TContent Content { get; }

///
/// Gets the content of the message.
///
/// Returns the message content
new TContent GetContent();
}

public abstract class MessageBase : IMessage where TContentType : class
{
///
/// Gets the content of the message.
///
public TContentType Content { get; protected set; }

///
/// Gets the content of the message.
///
///
/// Returns the message content
///
public TContentType GetContent()
{
return this.Content;
}

///
/// Gets the content.
///
///
/// Returns the content of the message
///
object IMessage.GetContent()
{
return this.GetContent();
}
}

public class ShoutMessage : MessageBase
{
///
/// Initializes a new instance of the class.
///
/// The message.
public ShoutMessage(string message)
{
this.Content = message;
}
}

///
/// Used when an object needs to send a private message to a character.
///
public class WhisperMessage : Messa

Solution

object GetContent();
TContent Content { get; }
new TContent GetContent();


How is GetContent() (both versions) useful? The non-generic IMessage could be just a marker interface, with no methods.

I'm not even sure about the usefulness of Content, since it seems it doesn't contain all the content of the message. E.g. treating WhisperMessage as a IMessage won't give you everything.

if (this.condition == null)
{
    this.callback(message, this);
    return;
}

if (!this.condition(message))
{
    return;
}

this.callback(message, this);


This is unnecessarily complicated, you could simplify it to:

if (this.condition == null || this.condition(message))
{
    this.callback(message, this);
}


List subscribers = listeners[messageType];
lock (listenerCollectionLock)
{
    subscribers.Add(handler);
}


You could lock here (and in Unsubscribe()) on subscribers. That way, adding subscribers for two different message types could happen at the same time.

Code Snippets

object GetContent();
TContent Content { get; }
new TContent GetContent();
if (this.condition == null)
{
    this.callback(message, this);
    return;
}

if (!this.condition(message))
{
    return;
}

this.callback(message, this);
if (this.condition == null || this.condition(message))
{
    this.callback(message, this);
}
List<ISubscription> subscribers = listeners[messageType];
lock (listenerCollectionLock)
{
    subscribers.Add(handler);
}

Context

StackExchange Code Review Q#77855, answer score: 3

Revisions (0)

No revisions yet.