aspose file tools*
The moose likes Java in General and the fly likes Avoiding too many if..elseif...else statements. Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Java in General
Bookmark "Avoiding too many if..elseif...else statements." Watch "Avoiding too many if..elseif...else statements." New topic
Author

Avoiding too many if..elseif...else statements.

Mark Waes
Greenhorn

Joined: Aug 06, 2008
Posts: 21
Hi,

I've written a small program which connects to a game server and listens for events published as things occur in the game. The server outputs a specific 'packet' format, which esentially can be thought of as an ArrayList containing Strings, with the first element being the event type, and the following elements being the event's parameters. There are 15 events currently (though more could be introduced) and each event can have a different number and type of parameters. Examples are:

  • OnJoin: The name of player who has just joined.
  • On Authenticated: The name of the player authenticated, and their GUID.


  • The above two are fairly simple, there are some events which returns lists of players with all their associated game stats (32 players x 9 different types of stat from memory).

    I was thinking of taking the packets mentioned previously and looking at the first element to see what kind of event had just occurred, then returning an object encapsulating that event, for example AuthenticatedPlayer would be a and object containing two strings for the name and GUID.

    However, to do this the first solution which comes to my mind is a longish list of if...elseif statement comparing Strings to establish what kind of event has occurred so the correct type of object can be instantiated. This didn't seem ideal to me as the if...elseif list would have to be maintained if new events were introduced and it doesn't seem a particularly elegant solution.

    Does anyone have any pointers as to alternative direction I could look at, or is the if...elseif (x14) a good approach?



    Arek Sokolowski
    Greenhorn

    Joined: Jan 12, 2011
    Posts: 8

    Use Enum as event type; then you can use switch-case instead of if-else construction.
    Of course, you will have to rebuild your data format; List<String> will be not enoug any more. Use your own class instead.
    Ralph Cook
    Ranch Hand

    Joined: May 29, 2005
    Posts: 479
    Just another possibility:

    Create a hashmap; key is the event type string returned from your server, value is an instance of a class to process that kind of message. Each such class implements an interface, perhaps with the method processEvent(String eventString).

    Then your event processing takes the key, looks up the class in the hash, and passes the string to the value class it finds there. No ifs (or ands or butts 8>). That value class understands how to process the corresponding event string.

    Adding an event requires you to add an entry to the hash.

    If you need a separate instance for each message, you could store the NAME of the class in the hash, instantiate it with class.forName(), and still process it as above.

    rc
    Mark Waes
    Greenhorn

    Joined: Aug 06, 2008
    Posts: 21
    Thanks for the replies all.

    I did consider the enum/switch statement approach but I felt that wasn't much of an improvement over the elseifs since they're the same thing in principle. I'm of the opinion (thought I can't remember where from), that large elseif/switch should be avoided if possible. Since my posting this topic I stumbled across this interesting video on you tube: The Clean Code Talks -- Inheritance, Polymorphism, & Testing, where the speaker discusses structuring code in such a way as to minimise if statements. I think it is slightly related and interesting - I thought I would share.

    I did also consider the map idea, but then I wasn't sure how to instantiate the required objects at run-time. I'll take a look at the Class.forName approach - I've not used this before, so it will be good to get my hands dirty on something new.

    Ralph Cook
    Ranch Hand

    Joined: May 29, 2005
    Posts: 479
    Be sure you actually need a new instance per message; I suppose in a multiplayer game it might be necessary, but if you don't need it you can save a lot of run-time cycles by not instantiating with every message.

    Here's another idea: an abstract static method in an abstract class that your classes extend. Look up your class in the hash, call the static method, and it can instantiate if it needs to; it can even instantiate its own class! So each static method would be specific to processing one type of event, and you could instantiate when you needed to and not if you didn't.

    Won't have to use forName(), in that case.

    rc
    Mark Waes
    Greenhorn

    Joined: Aug 06, 2008
    Posts: 21
    Hi Ralph,

    Yes it is a multi-player game and the events are typically centred around the players on it or the round/map just played. Obviously these things change frequently, so I would've thought it difficult to avoid instantiation in 99% of cases.

    Back on topic... It seems static abstract methods aren't allowed, which makes sense I suppose, since abstract methods are supposed to be overridden. Many thanks for your suggestions though; you've definitely given me some new angles to explore.

    Ralph Cook
    Ranch Hand

    Joined: May 29, 2005
    Posts: 479
    My apologies for misleading you -- I carefully checked to make sure that abstract classes could contain static methods, and forgot to make sure that those methods could be static.

    So if you're going to instantiate 99% of the time anyway, you don't have to worry about that. If you were going to worry about that, then you might use a single class with a static "factory method", so it could decide whether (and what) to instantiate. And it could use something like the hash table I mentioned originally in the cases where it was going to instantiate.

    Good luck with your program.

    rc
    William Brogden
    Author and all-around good cowpoke
    Rancher

    Joined: Mar 22, 2000
    Posts: 12806
        
        5
    I felt that wasn't much of an improvement over the elseifs since they're the same thing in principle.


    Wrong! Switch/case takes the same amount of time no matter which case is selected while elseifs have way too many if evaluations to reach later statements.

    Makes a big difference.

    Bill
    Mark Waes
    Greenhorn

    Joined: Aug 06, 2008
    Posts: 21
    Thanks for the advice Bill.

    When I wrote that I wasn't thinking of it from a performance perspective, but now you mention it, it is worth considering.

    I may go the switch route eventually: this project is as much for the personal learning experience and fun as it is for producing something workable. I guess I have the luxury of not being bound to a customer's requirements/deadlines as well as balancing productivity with other factors.

    When it comes down to it though, I will have to implement something to get it off the ground, so I will bear in mind your advice.

    Many thanks.
    William Brogden
    Author and all-around good cowpoke
    Rancher

    Joined: Mar 22, 2000
    Posts: 12806
        
        5
    In my experience, switch/case results in much more understandable code and is easier to expand for new functions.

    Bill
     
    I agree. Here's the link: http://aspose.com/file-tools
     
    subject: Avoiding too many if..elseif...else statements.