# Conditional execution based on input string's prefix

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!";

//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";
}

-

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.

-
I like the idea of creating new classes to handle the commands because what I really want is it get rid of those nasty if-statements. As for the return type it wouldn't really help in this scenario to change it because of how the rest of the application works. –  warbio Jan 10 '13 at 15:51

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.

-
The problem is that the class already is Observable so I feel it would just add confusion to make it an Observer aswell(if even possible). –  warbio Jan 10 '13 at 15:41
@warbio no it is not a problem : for Java it is powerful to be an observer observable for managing nested/complex logical choices. To help maintenance you can put observers in a sub-package of observable, so hierarchy is visible at the first glance. –  cl-r Jan 10 '13 at 15:51

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";
}

-
Returning "-1" still falls into the "stringly typed" antipattern. –  200_success Jan 17 '14 at 7:26