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?
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.
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.
Joined: Aug 06, 2008
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.
Joined: May 29, 2005
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.
Joined: Aug 06, 2008
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.
Joined: May 29, 2005
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.
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.
Author and all-around good cowpoke
Joined: Mar 22, 2000
In my experience, switch/case results in much more understandable code and is easier to expand for new functions.
subject: Avoiding too many if..elseif...else statements.