5
\$\begingroup\$

I wrote a parser for the IRC RFC 2812 spec and would like input on my use of the strategy pattern for this.

I created an IrcMessage class that is responsible for taking the server response string from the (examples below) and parsing it. The server response format is:

:<Prefix> <Command> <Space Separated Args> :Optional trailing content here, like a chat message.

Examples:

:[email protected] PRIVMSG #mychannel :Hello everyone!
:[email protected] 301 Bob :Away playing a game
:Scionwes[email protected] 305 :You are no longer marked as away

This class takes the string and passes it to a MessagePrefix, MessageTrail and MessageCommand class. Each parsing the string and building their respective component.

/// <summary>
/// Breaks an IRC server response down in to a prefix, command w/ parameters and an optional trail.
/// Since the response format is always :<prefix> <Command> <arg, arg, arg ...> :trailing content
/// the Parser can easily break the response down in to objects representing each component of
/// the message.
/// </summary>
public class IrcMessage
{
    /// <summary>
    /// Initializes a new instance of the <see cref="IrcMessage"/> class.
    /// </summary>
    /// <param name="message">The message.</param>
    public IrcMessage(string message)
    {
        this.OriginalMessage = message;
        this.PrefixMessage = new MessagePrefix(message);
        this.TrailMessage = new MessageTrail(message);
        this.CommandMessage = new MessageCommand(message, this.PrefixMessage, this.TrailMessage);
    }

    /// <summary>
    /// Gets the original message sent from the server.
    /// </summary>
    public string OriginalMessage { get; private set; }

    /// <summary>
    /// Gets the prefix of the response.
    /// </summary>
    public MessagePrefix PrefixMessage { get; private set; }

    /// <summary>
    /// Gets the trailing message. Not all responses come with trailing content.
    /// </summary>
    public MessageTrail TrailMessage { get; private set; }

    /// <summary>
    /// Gets the command and its parameters sent from the server.
    /// A command will always be given; parameters might be empty.
    /// </summary>
    public MessageCommand CommandMessage { get; private set; }
}

/// <summary>
/// Parses a server response for a prefix and stores it.
/// </summary>
public class MessagePrefix
{
    /// <summary>
    /// The original message
    /// </summary>
    private string originalMessage;

    /// <summary>
    /// Initializes a new instance of the <see cref="MessagePrefix"/> class.
    /// </summary>
    /// <param name="message">The message.</param>
    public MessagePrefix(string message)
    {
        this.originalMessage = message;

        if (!this.IsPrefixed)
        {
            return;
        }

        // Prefix always starts with a colon and ends with a space.
        // :<Prefix> <Command>
        this.EndIndex = message.IndexOf(" ");
        this.Prefix = message.Substring(1, this.EndIndex - 1);
    }

    /// <summary>
    /// Gets a value indicating whether the original message contained a properly formatted prefixed.
    /// </summary>
    public bool IsPrefixed
    {
        get
        {
            return this.originalMessage.StartsWith(":");
        }
    }

    /// <summary>
    /// Gets the index at the end of the prefix.
    /// </summary>
    public int EndIndex { get; private set; }

    /// <summary>
    /// Gets the prefix of the message. This is all text after the initial colon and before the first space.
    /// </summary>
    public string Prefix { get; private set; }
}

/// <summary>
/// Parses an IRC server response for the command and its arguments.
/// </summary>
public class MessageCommand
{
    /// <summary>
    /// Initializes a new instance of the <see cref="MessageCommand"/> class.
    /// </summary>
    /// <param name="message">The message.</param>
    /// <param name="prefix">The prefix.</param>
    /// <param name="trail">The trail.</param>
    public MessageCommand(string message, MessagePrefix prefix, MessageTrail trail)
    {
        // Remove all of the content after the Prefix but before the Trail.
        // That is our Command and it's associated arguments.
        // Formatted as
        // :<Prefix> <Command> <space separated args> :<Trail>
        string[] commandAndParameters = message
            .Substring(prefix.EndIndex + 1, trail.TrailStart - prefix.EndIndex - 1)
            .Split(' ');

        // First item is always the command.
        this.Command = commandAndParameters[0];

        // If the command as args, then they compose the rest of the collection.
        if (commandAndParameters.Length > 1)
        {
            this.Parameters = commandAndParameters.Skip(1);
        }
    }

    /// <summary>
    /// Gets the command sent by the server.
    /// </summary>
    public string Command { get; private set; }

    /// <summary>
    /// Gets the parameters associated with the command, if any..
    /// </summary>
    public IEnumerable<string> Parameters { get; private set; }
}

/// <summary>
/// Parses an IRC server response for trailing content.
/// </summary>
public class MessageTrail
{
    /// <summary>
    /// Initializes a new instance of the <see cref="MessageTrail"/> class.
    /// </summary>
    /// <param name="message">The message.</param>
    public MessageTrail(string message)
    {
        // Find the index of the closing colon used to mark the end of the 
        // message meta and start of the trailing content.
        // This is always the first space, followed by a colon in the response.
        this.TrailStart = message.IndexOf(" :");
        if (this.TrailStart >= 0)
        {
            // Set the trailing content to the location were the trailing content begins.
            // The +2 is needed to begin after the ' :' index.
            this.TrailingContent = message.Substring(this.TrailStart + 2);
        }
        else
        {
            this.TrailStart = message.Length;
        }

        this.HasTrail = this.TrailStart > 0 && this.TrailStart < message.Length;
    }

    /// <summary>
    /// Gets a value indicating whether the server response has trailing content.
    /// </summary>
    public bool HasTrail { get; private set; }

    /// <summary>
    /// Gets the trailing content of the response.
    /// </summary>
    public string TrailingContent { get; private set; } = string.Empty;

    /// <summary>
    /// Gets the string index from the original message where the trailing content starts.
    /// </summary>
    public int TrailStart { get; private set; }
}

The following is my ICommandResponse, which is what I am using as my strategy pattern object. Each implementation of this correlates to a server code (or series of codes/commands) that can be processed with a result given back for the client UI to consume.

/// <summary>
/// Process a command sent from the server.
/// </summary>
public interface ICommandResponse
{
    /// <summary>
    /// Gets the command or Reply codes associated with this response.
    /// </summary>
    IEnumerable<string> CommandCodes { get; }

    /// <summary>
    /// Processes the command.
    /// </summary>
    /// <param name="message">The message.</param>
    /// <param name="connection">The connection.</param>
    /// <param name="userInfo">The user information.</param>
    /// <param name="args">The arguments.</param>
    /// <returns>
    /// Returns the output result of the process.
    /// </returns>
    string ProcessCommand(IrcMessage message, IClientConnection connection);
}

I take the content sent to me from the server and pass it to an IrcMessage instance via its constructor. I then pass that instance to a ProcessMessage method that finds an ICommandResponse object that can handle the command code that the server sent. I don't include the CommandResponseFactory code as it just searches for ICommandResponses that have the command given within the ICommandResponse.CommandCodes collection and returns the correct one.

private void ProcessMessage(IrcMessage message)
{
    ICommandResponse response = CommandResponseFactory.GetResponseForCommand(message.CommandMessage.Command);

    // temporary: we will just dump the message to the client.
    if (response == null)
    {
        this.OnMessageReceived(message.OriginalMessage);
        return;
    }

    string result = response.ProcessCommand(message, this);

    if (message.CommandMessage.Command.Equals("001"))
    {
        this.OnConnected(result);
    }
    else
    {
        this.OnMessageReceived(result);
    }
}

I've seen most people handle the response codes in a giant Switch(responseCode) statement. I didn't want to take that approach. With this, I can now take each one of the response codes that the server sends me, and handle them as objects.

A couple examples, the first being handling the Message of the Day response codes.

/// <summary>
/// Processes all incoming message of the day responses from the server
/// </summary>
internal class MessageOfTheDayReplies : ICommandResponse
{
    /// <summary>
    /// Gets the command or Reply codes associated with this response.
    /// </summary>
    public IEnumerable<string> CommandCodes { get; private set; } = new List<string> { "375", "376", "372" };

    /// <summary>
    /// Processes the command.
    /// </summary>
    /// <param name="command">The command.</param>
    /// <param name="code">The code.</param>
    /// <param name="connection">The connection.</param>
    /// <param name="userInfo">The user information.</param>
    /// <param name="args">The arguments.</param>
    /// <returns>Returns back messages associated with the message of the day.</returns>
    public string ProcessCommand(IrcMessage message, IClientConnection connection)
    {
        /* Expected server responses for each supported response code.
            375    RPL_MOTDSTART
                ":- <server> Message of the day - "
            372    RPL_MOTD
                ":- <text>"
            376    RPL_ENDOFMOTD
                ":End of MOTD command"

           When responding to the MOTD message and the MOTD file
           is found, the file is displayed line by line, with
           each line no longer than 80 characters, using
        */

        if (message.CommandMessage.Command.Equals("375"))
        {
            return message.TrailMessage.HasTrail
                ? message.TrailMessage.TrailingContent
                :  "<\{connection.ServerInformation.Url}>  Message of the Day - Begin";
        }
        else if (message.CommandMessage.Command.Equals("376"))
        {
            return message.TrailMessage.HasTrail
                ? message.TrailMessage.TrailingContent
                : "<\{connection.ServerInformation.Url}>  Message of the Day - End";
        }
        else
        {
            return message.TrailMessage.TrailingContent;
        }
    }
}

The 2nd being handling away replies.

/// <summary>
/// Processes away responses from the server
/// </summary>
internal class AwayReplies : ICommandResponse
{
    /// <summary>
    /// Gets the command or Reply codes associated with this response.
    /// </summary>
    public IEnumerable<string> CommandCodes { get; private set; } = new List<string> { "301", "305", "306" };

    /// <summary>
    /// Processes the command.
    /// </summary>
    /// <param name="command">The command.</param>
    /// <param name="code">The code.</param>
    /// <param name="connection">The connection.</param>
    /// <param name="userInfo">The user information.</param>
    /// <param name="args">The arguments.</param>
    /// <returns>Returns the away message for the user messaged.</returns>
    public string ProcessCommand(IrcMessage message, IClientConnection connection)
    {
        /* Expected server responses for each supported response code.
        301    RPL_AWAY
            "<nick> :<away message>"
        305    RPL_UNAWAY
            ":You are no longer marked as being away"
        306    RPL_NOWAWAY
            ":You have been marked as being away"

           These replies are used with the AWAY command (if allowed).  
           RPL_AWAY is sent to any client sending a PRIVMSG to a client which is away.  
           RPL_AWAY is only sent by the server to which the client is connected.
           Replies RPL_UNAWAY and RPL_NOWAWAY are sent when the
           client removes and sets an AWAY message.
        */
        string response = string.Empty;

        if (message.CommandMessage.Command.Equals("301"))
        {
            var user = message.CommandMessage.Parameters.First();
            response = message.TrailMessage.HasTrail 
                ? "\{user} is away with message '\{message.TrailMessage.TrailingContent}'"
                : "\{user} is away";
        }
        else if (message.CommandMessage.Command.Equals("305"))
        {
            response = message.TrailMessage.HasTrail
                ? message.TrailMessage.TrailingContent
                : "You are no longer marked as being away.";
        }
        else if (message.CommandMessage.Command.Equals("306"))
        {
            response = message.TrailMessage.HasTrail
                ? message.TrailMessage.TrailingContent
                : "You have been marked as being away.";
        }

        return response;
    }
}

I wrote a unit test that checks to ensure the parser parsed the data properly. I've got a few other tests for different string combinations, but this one was covered all of the IrcMessage in a single test for a complete response string.

    [TestMethod]
    public void Parse_with_trailing_content()
    {
        // Arrange
        string prefix = ":irc.localhost.localdomain";
        string commandAndArgs = "433 Scionwest";
        string trailingContent = " :Nickname is already in use";

        // Act
        var parser = new IrcMessage("\{prefix} \{commandAndArgs} \{trailingContent}");

        // Assert
        // We +2 the expected value to account for spaces used in the string formatting.
        Assert.AreEqual(prefix.Length + commandAndArgs.Length + 2, parser.TrailMessage.TrailStart, "Trailing index was not found.");
        Assert.IsTrue(parser.TrailMessage.TrailingContent.Equals(trailingContent.Substring(2)), "Trailing content not parsed.");
        Assert.AreEqual(prefix.Length, parser.PrefixMessage.EndIndex, "Prefix ending index not found.");
        Assert.IsTrue(parser.PrefixMessage.Prefix.Equals(prefix.Substring(1)), "Prefix was not equal");
        Assert.IsTrue(parser.CommandMessage.Command.Equals(commandAndArgs.Split(' ').First()), "Command was not parsed.");
        Assert.IsTrue(parser.CommandMessage.Parameters.First().Equals(commandAndArgs.Split(' ').Last()), "Command args not parsed.");
    }
Improve this question
\$\endgroup\$

3 Answers 3

Reset to default
5
\$\begingroup\$

Structure

Your object structure seems a little off to me. For example:

  • Why does MessageTrail have a HasTrail property? It is the trailing content.
  • Why does MessagePrefix have a IsPrefixed property? It is the prefix.
  • There are loads of public properties that don't seem to add anything.
  • I don't think your classes are "sharp" enough - the lines between separate responsibilities are blurred to me.

I would expect only 2 classes - a Message class and a Command class. The Message would encapsulate the entire message and the Command would encapsulate the command and parameters.

Here's how I'd expect them to look (omitting code for parsing):

public class IrcMessage
{
    public string Prefix { get; private set; }
    public string TrailingContent { get; private set; }
    public IrcCommand Command { get; private set; }

    public bool HasPrefix 
    {
        get
        {
            return !string.IsNullOrEmpty(Prefix);
        }
    }

    public bool HasTrailingContent 
    {
        get
        {
            return !string.IsNullOrEmpty(TrailingContent);
        }
    }
}

public class IrcCommand
{
    public string CommandText { get; private set; }
    public IEnumerable<string> Parameters { get; private set; }
    public bool HasParameters 
    {
        get
        {
            return Parameters != null && Parameters.Any();
        }
    }
}

Comments

Nitpicking but I don't think class documentation should say it is doing something. E.g. 

/// <summary>
/// Parses a server response for a prefix and stores it.
/// </summary>
public class MessagePrefix

No it doesn't. MessagePrefix represents the prefix of an IRC message. The constructor is the thing that is parsing a message for a prefix.

/// <summary>
/// Gets the command or Reply codes associated with this response.
/// </summary>
public IEnumerable<string> CommandCodes { get; private set; }

Which one is it?

General

Wouldn't it be easier to use if your prefix and trailing content didn't include the colon (:)?

This looks like that rare problem that would be easier to solve with regular expressions!

Improve this answer
\$\endgroup\$
6
  • \$\begingroup\$ To answer your first two questions, the IRC spec requires a prefix on all responses but allows trailing content to be optional. I've encountered two servers so far in testing however that omit the prefix on certain non-spec'd messages they send. So rather than doing a IsNullOrWhitespace check when looking for that info, I hit the property. \$\endgroup\$
    – Johnathon Sullinger
    Feb 6, 2015 at 14:46
  • \$\begingroup\$ Thanks for the input on splitting the command and parsed message. I like that idea. In response to your last two sentences: The trailing colon is a required by an IRC server according to the IRC RFC. It would certainly make my life easier if I didn't have to parse through it, but since it's in the spec I have to. \$\endgroup\$
    – Johnathon Sullinger
    Feb 6, 2015 at 14:50
  • \$\begingroup\$ The spec says the prefix is optional. "The prefix is used by servers to indicate the true origin of the message. If the prefix is missing from the message, it is assumed to have originated from the connection from which it was received from." \$\endgroup\$
    – RobH
    Feb 6, 2015 at 14:53
  • \$\begingroup\$ The point I was making with the first two questions is that the trail object is not the right place to check whether the message contains a trail. That should be the job of the message IMO. \$\endgroup\$
    – RobH
    Feb 6, 2015 at 14:54
  • \$\begingroup\$ Interesting. It makes more sense when you remove the prefix and trail content all together and just have IrcMessage do the work. I wouldn't need either object at all. \$\endgroup\$
    – Johnathon Sullinger
    Feb 6, 2015 at 15:10
4
\$\begingroup\$

You should use an auto property for IsPrefixed which should have a private (or proteced) setter which is filled inside the constructor. 

public bool IsPrefixed { get; private set; }

public MessagePrefix(string message)
{

    if (!this.IsPrefixed = message[0] == ':')
    {
        return;
    }

    // Prefix always starts with a colon and ends with a space.
    // :<Prefix> <Command>
    this.EndIndex = message.IndexOf(" ");
    this.Prefix = message.Substring(1, this.EndIndex - 1);
}

this also removes the need to keep the passed message.

The constructor of the MessageTrail class can be simplified like 

public MessageTrail(string message)
{
    // Find the index of the closing colon used to mark the end of the 
    // message meta and start of the trailing content.
    // This is always the first space, followed by a colon in the response.
    this.TrailStart = message.IndexOf(" :");
    if (this.TrailStart >= 0)
    {
        // Set the trailing content to the location were the trailing content begins.
        // The +2 is needed to begin after the ' :' index.
        this.TrailingContent = message.Substring(this.TrailStart + 2);
        this.HasTrail = true;
    }
}

this will remove the condition at the end and also the need to set the TrailStart if no colon had been found. 

/// <summary>
/// Process a command sent from the server.
/// </summary>
public interface ICommandResponse

This xml documentation explains that the given name ICommandResponse isn't the best. One would expect e.g get a response from a processor. So a better name would be ICommandProcessor. 

/// <summary>
/// Processes the command.
/// </summary>
/// <param name="message">The message.</param>
/// <param name="connection">The connection.</param>
/// <param name="userInfo">The user information.</param>
/// <param name="args">The arguments.</param>
/// <returns>
/// Returns the output result of the process.
/// </returns>
string ProcessCommand(IrcMessage message, IClientConnection connection);

here the documentation is lying. It talks about the parameters userInfo and args which aren't there.

While we are at this method declaration, you don't use the connection in any of your samples. Maybe you should remove it.

You aren't using the CommandCodes property neither.

A construct like 

if(condition)
{
    // some code
    return someValue;
}
else if (someOtherCondition)
{
    // some code
    return someOtherValue;
}
else
{
    // some code
    return someOtherValue;
}

makes the else part redundant.

Improve this answer
\$\endgroup\$
1
  • \$\begingroup\$ Thanks, I'll take a look at simplifying those two constructors. It seems I forgot to cleanup that method after removing the connection and user info usages. The CommandCodes are not used by the implementation itself, but rather it's used by a Factory to determine what ICommandResponse needs to be returned back to the ClientConnection to process the server response. This could be replaced with Attributes. At the time I wanted to avoid reflection, but this would probably be a good use case for them. \$\endgroup\$
    – Johnathon Sullinger
    Feb 6, 2015 at 14:44
2
\$\begingroup\$

IRC can get a little hectic. Storing the variables in the constructor during initialization makes more sense instead of breaking the string down every time you need a variable.

Improve this answer
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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