I fully realize that I am 2 years late to the party 🙂 but probably few clarifications will help other devs.

What the other answers missed to address was the mentioning of the “Kryonet” which adds additional twist to the problem: usually when kryonet is used it means that

a) the client is an android app and

b) the client is some game

If both conditions are met IMHO using the instanceof if-else sequence is the way to go.

The usual way of structuring client/server app in our case android game + “pure” java server is to use (at least) 3 projects:

• “common” – which contains code common for the server and the client, most importantly all the message classes (which usually implement some tag/marker interface as GameMessage).
• server – contains the server and includes the common project. Usually all messages are simply registered in Kryonet, i.e. no pooling is used (because on server GC is not considered a problem)
• android client – contains the android app (game) and includes the common project. Because in java games creating objects during the game may trigger the GC it is considered the biggest mortal sin (which leads straight to the hell of missed frames) it is considered good practice¹ to use object pooling for the game messages (this requires little customization on the Kryonet/kryo in order to use object pools). All messages are (must be) registered in kryonet in the same order as on the server in order things to work.

First, lets look at the alternatives and see why they are not fit for the structure/implementation described above:

1. They say ‘Use polymorphism and method overriding’

Take a look here for an example of the proposed solution (accepted answer by jmg). It looks very elegant but there there are at least 2 big problems:

1. You can’t just use parameterless overrided method like do() because in real implementation you will some need external data to act upon. For example when receiving PlayerAbandonedGame message on the server you will need at least the particular game object as parameter in order to call removePlayer(Player pl) method. NewGameState message will require reference to the particular GameState variable that will be updated and so on. One very ugly solution is overrided method to have single parameter like TheWholeThing which contains all the objects that may be needed. It is needless to say that this not only violates the principle of encapsulation but also “spreads” the game logic across multiple message classes.

2. Remember that we have the common project and all the messages are defined there. Even if server-to-client messages are completely distinct set to client-to-server messages it will be simply stupid to put server or client logic in the common project.

Game message objects are just data containers. It is up to the server or the client to handle that data in a particular way (or even ignore it as often happens with out of order UDP messages).

2. They say ‘Use reflection’

Our context is ‘a) the client is an android app’. The reaction of every android developer hearing Use reflection should be to stand up, clear his/hers throat and with his/hers best british accent to say: ‘Are you completely mAd?!’ 🙂

Reflection is very slow on older Android versions, it is slow even now (5.0). Also IMO using reflection is the ugliest possible solution not just for this particular problem. It should be avoided up in the stack, i.e. in app dev.

Way to go

Back to the original question: if one really insist on using switch the solution is to have base class like:

public abstract class GameMessage {
    byte mCode; // not private because of kryo

    public GameMessage(byte code) {
        super();
        mCode = code;
    }

    public byte getCode() {
        return mCode;
    }
}

concrete message will look like:

public class PlayerAbandonedGame extends GameMessage {
    // some fields

    public PlayerAbandonedGame() {
        super((byte) 42);
    }
}

and then message handler will look like:

public synchronized void onMessageReceived(GameMessage msg) {
    switch(msg.getCode()) {
        case 42:
            // do something
            break;
        default:
            mLogger.error("Unknown message code {}", msg.getCode());
            // may be even throw an exception?
            break;
    }

    // ...
}

Of course it is advisable to use enum values instead of hardcoded numbers…

Potential drawback is (as mentioned in other answers) if you create new message class and forget to add it to some of the switch statements. The compiler cannot catch this problem and you will hit it only runtime (in the default case).

Real (small) drawback is that code field is send each time and adds to the traffic. In reality this adds about 1-10% to the size of the messages (if you use byte for the code field).

How I do it

I personally use if-else sequence with instanceof test like:

public synchronized void onMessageReceived(GameMessage msg) {
    if (msg instanceof MyLetterAttemptResult) {
        handleMyLetterAttemptResult((MyLetterAttemptResult) msg);
        mPools.release(msg);
    } else if (msg instanceof InAutoMatchQueue) {
        handleInAutoMatchQueue((InAutoMatchQueue) msg);
    } else if (msg instanceof AutoMatchRetry) {
        handleAutomatchRetry();
    // more else-ifs
    } else {
        mLogger.error("Unknown message {}", msg.getClass().getSimpleName());
        // throw exception may be?
    }
}

Isn’t that “code smell”? You bet it is, but after trying all other alternatives it seems to me that it is best. To reduce the smelliness it is better to try to keep code in the ifs short like above. Just call handle* method and release message back to the pool (if it is pooled).

Isn’t that slow? It is a bit slower than the switch(msg.getCode()) but not that much. Here is some benchmarking (answer by Dan Mihai Ile). Advantage is that you don’t have the code field send each time over the network. Keep the most frequently used messages at the top so they are found first.

Isn’t that error prone? There is a risk if you create new message class and forget to add corresponding else if. The thing is that you have just two separate message handling methods – one on the server and one on the client. Both are the single most important code pieces repectively on the server and the client. You will simply have to not mess it up. Just add the new message class immediately to the unit test that tests onMessageReceived().

This is not OOP! I have a better solution! I will be very happy to discuss it and use it :-). btw, I wonder if @Cameron (the author of this question) found better solution. After all this question is 2 years old…

¹ For me avoiding the GC is more like “utter crap” than “best practice”. As Eonil commented in 1: I regard avoiding GC on GC based language is a kind of insane trial.

The answer is no.

But using Java 7 you could do this:

switch (object.getClass().getName()) {
  case "some.pkg.SecondObject":
    break;
  case "some.pkg.ThirdObject":
    break;
}

… which is not exactly the same, but may be close enough.

The problem is that this is “code smell”, whether you do it using if ... instanceof tests or the switch hackery above.

Why?

Because any time you add a new class, you need to revisit all of the places where you do this class testing!! And there is the added problem that the code breaks if you rename the classes.

The recommended OO approach is to use polymorphism.

Anything added to the object increases the size of these packets, so to minimize the network traffic, you keep these objects as light weight as possible.

Methods don’t add anything to the runtime size of an object. The method code is not transmitted, and nothing about the methods requires any representation in an object’s state. This is not a sound reason for instanceof testing, etc.

No, and it’s very unlikely that such a thing will be added.

You see, checking for the type of an Object and then doing different things for different types is considered terribly inelegant, because there already is a much better pattern for doing that: using inheritance and overridden methods. (Granted, that’s only of use when the “do something” can be formulated as a method call on the checked Object, but usually it can.)

In Java, it’s only possible to switch on primitives. But you could do something similar.
I think it’s some kind of design pattern though I could not find it…

You could hide the if-then-else stuff (which is needed in this case) and wrap it in a kind of switch-class:

public abstract MySwitch{
    public MySwitch(MyObject obj){

        if (obj instanceof SecondObject)
            secondObject((SecondObject)obj);
        else if (obj instanceof ThirdObject)
            thirdObject((ThirdObject)obj);
        else
            onDefault(obj);
    }

    protected abstract void secondObject(SecondObject obj);
    protected abstract void thirdObject(ThirdObject obj);
    protected abstract void onDefault(MyObject obj);
}

You could use it then like this in your code:

new MySwitch(obj){

    @Override protected void secondObject(SecondObject obj){

    }
    @Override protected void thirdObject(ThirdObject obj){

    }
    @Override protected void onDefault(MyObject obj){

    }
};

Just be aware that normally, calling abstract methods from constructor is a bad idea… I think, in this case, it could be OK, because this is and always will be a stateless class. If you don’t like it, add an additional method to do the stuff.

This also has the advantage that if you add new classes to your inheritance tree and add a new case, you are forced to update all occurrences and don’t forget a switch somewhere.

I think we can do in this way too:

public enum Switch {
  BED_ROOM, BATH_ROOM, LIVING_ROOM 
}

then create an interface Switchable:

public interface Switchable {
  public Switch getSwitch();
}

Then implement switches that take switch type as constrictor param:

public BedRoom implements Switchable {

  private Switch switch;

  public BedRoom(Switch switch) {
    this.switch = switch;
  }  

  public Switch getSwitch() {
    return switch;
  }

  public int getBedCapacity() {
    return 2; // implement the way your wish
  }
}

Then use like below:

Switchable switch = new BedRoom(Switch.BED_ROOM)
switch(switch.getSwitch()) {
  case BED_ROOM:
    BedRoom br = (BedRoom) switch;

    br.getBedCapacity(); // do anything with bedroom methods.
    break;
}

If there is a common base class, then you can add a method there returning some enum value, and override it in each subclass. So you can switch on “object.type” for example without any problems. You can add an assert at the end of the switch statement, and in some languages the compiler will warn you if you added a new type to the enum and don’t take it into account,