Is there a cleaner way to write this method?

Question

I have this method that basically takes a string and does different things depending on what the string starts with. Is there a cleaner way to write this method?

public String analyze(BotMessage message)
{

    String lcmsg = message.message().toLowerCase();
    //Not a command
    if(!lcmsg.startsWith("!")) return "-1";

    //Message from whatever
     if(lcmsg.startsWith("!ping")) return "pong!";
     if(lcmsg.startsWith("!tournament")) return tournament(message);


     //message from skype
     if(message.skype())
     {
         if(lcmsg.startsWith("!settournament")) return settournament(message);
         if(lcmsg.startsWith("!promote")) return promote(message);
         if(lcmsg.startsWith("!checkin")) return checkin(message);
         if(lcmsg.startsWith("!checkout")) return checkout(message);
         if(lcmsg.startsWith("!checkedin")) return checkedin(message);
         if(lcmsg.startsWith("!updatetournament")) return updatetournament();

     }

    return "-1";
}

Answer 1

I don't like too much the fact that you use a String as the returned type of your function.

What about using a class hierarchy or exceptions?

I see opportunities to create a hierarchy even for the messages. Why don't you create a BotMessage and a SkypeMessage?

Both should have the logic to decide what to do in the base case, maybe in an analyze method. The analyze method of the SkypeMessage should, in addition, manage the Skype-specific aspects.

Finally you should simplify the code and remove the list of if statements. Introduce a new set of objects to parse the messages. These objects should have a match method that check whether the string matches with their definition, and a performAction method that generates the return value.

Answer 2

You can use Observer/Observable pattern for the if(message.skipe(){..} area.

So each each observer evaluate the if and run the method (the methods are wrote inside the Observer).

If a new test is needed, you have just to had a new Observer with no impact on other Observers.

Many useful patterns are very clearly explained in O'Reilly "Head First Design Patterns" book.

Answer 3

For anyone interested I managed to rewrite the method following @mariosangiorgio advice. I made a Command interface and gave each command a own class.

public String analyze(Message message)
{       
    //Not a command
    if(!message.getMessage().startsWith("!")) return "-1";

    for(Command cmd : commands)
    {
        if(cmd.Match(message)) return cmd.preformAction(message);
    }

    return "-1";
}