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

IRC Server Response Parser for IRC Client

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

Problem

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:

:   :Optional trailing content here, like a chat message.


Examples:

:Scionwest!Scionwest@555.55.55.555 PRIVMSG #mychannel :Hello everyone!
:Scionwest!Scionwest@555.55.55.555 301 Bob :Away playing a game
:Scionwest!Scionwest@555.55.55.555 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.

```
///
/// Breaks an IRC server response down in to a prefix, command w/ parameters and an optional trail.
/// Since the response format is always : :trailing content
/// the Parser can easily break the response down in to objects representing each component of
/// the message.
///
public class IrcMessage
{
///
/// Initializes a new instance of the class.
///
/// The message.
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);
}

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

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

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

///
/// Gets the command and its parameters sent from the server.
/// A command will always be given; parameters might be empty.

Solution

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 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.

/// 
/// Parses a server response for a prefix and stores it.
/// 
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.

/// 
/// Gets the command or Reply codes associated with this response.
/// 
public IEnumerable 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!

Code Snippets

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();
        }
    }
}
/// <summary>
/// Parses a server response for a prefix and stores it.
/// </summary>
public class MessagePrefix
/// <summary>
/// Gets the command or Reply codes associated with this response.
/// </summary>
public IEnumerable<string> CommandCodes { get; private set; }

Context

StackExchange Code Review Q#78713, answer score: 5

Revisions (0)

No revisions yet.