views:

84

answers:

4

I'm writing an application to receive SMS messages via an HTTP gateway. The main parameters I'll be processing are the sender's phone number and the message itself. However, depending on the keyword within the message, I'll need to execute different logic / return a different response. To start with, I used a simple if/else which turned into a switch statement, and now I'm looking to clean it up a bit.

I think what want is some sort of implementation of the command pattern, but I'm not sure. I could implement each operation as a command object and register them all with the command processor, but then I'd have to implement something like a CanExecute(...) function to determine the appropriate command to execute. The conditions for which a command should be executed may depend on several factors such as the keyword or specific message content.

I'm thinking something like the following:

public interface ISmsCommand 
{
    bool CanExecute(string sender, string message);
    string Execute(string sender, string message);
}

public class SmsHelpCommand : ISmsCommand 
{
    public bool CanExecute(string sender, string message)
    {
        return (message.ToLower().StartsWith("help"));
    }

    public string Execute(string sender, string message) 
    {
        return "For more info, visit...";
    }
}

public class SmsHttpHandler : IHttpHandler
{
    List<ISmsCommand> _commands = new List<ISmsCommand>();

    public bool IsReusable
    {
        get { return true; }
    }

    public void ProcessRequest(HttpContext context)
    {
        var sender = context.Request.Form[...];
        var message = context.Request.Form["Message"];

        foreach (var command in _commands) 
        {
            if (command.CanExecute(sender, message))
            {
                var response = command.Execute(sender, message);
                SendTextMessage(sender, response);
                break;
            }
        }
    }
}

Something about this code seems fishy to me, though. I think I don't like having to send the arguments to both the CanExecute function and the Execute function, but I'm not sure. Any thoughts?

+1  A: 

The parsing of the message should be done ahead of time before passing to the command. At least enough to determine which command is applicable. In other words, it should not be the responsibility of a command object to determine whether or not a message represents that command. The command's status method should only determine whether or not a particular command is applicable in a given state. You shouldn't have to ask each command if it wants the message.

I would have commands like GetBalanceCommand, TransferMoneyCommand, etc. Then have a simple string tokenizer parse the message and determine which command object to instantiate and pass the tokenized string to the command's CanExecute/Execute methods.

Josh Einstein
That would put me right back where I am now.. a bit long if/else or switch statement doing the parsing of the commands...
Chris
I should elaborate that the logic to determine which command needs to be executed is not as simple as say, 1 command per key word. If it were, I'd simply use a dictionary lookup to determine which command to execute.
Chris
+1  A: 

Have a look at the Chain of Responsibility pattern. http://www.dofactory.com/patterns/PatternChain.aspx. Or you can rename your Execute method to TryExecute and return a bool.

Mufaka
+1  A: 

Take a look at the Factory method command and the Command pattern.

monksy
+1  A: 

I would continue with the command pattern, but remember that patterns are just guidelines (though I imagine that goes without saying).

Though normally in the command pattern you supply it with something to execute it's command upon. Think of how the SqlCommand object works, you give it the SqlConnection and just tell it to execute. The command object uses the SqlConnection to do what is needed. You put your foot on the gas pedal and it moves all the linkages in order to make the car go faster. You don't push the gas pedal and then open the throttle yourself.

What I would suggest is that you either give the command the SendTextMessage function or object that allows for that functionality (think of the SqlConnection) in it's constructor. Or just give it with the Execute method along with the sender and the message. This gives you the ability to just allow the command to do it's job. While at the same time allowing you to constrain its ability to do that via the SendTextMessage functionality you provide it. It will do it if it can and if it cannot, because it's not supposed to given the message, it won't send the message.

David