Handling multiple packet types in Java 8

Question

I have a Netty-based game server implementation that handles 40 or so distinct packets with their own serialization structure, for brevity I'll refer to them as FooPacket, BarPacket, ... These packet types have been assigned consecutive numerical "packet types", which are unsigned bytes. For example, FooPacket is type 0x01, BarPacket is type 0x02, etc...

All of these packets extend an abstract class, MyGameNetworkPacket.

My current approach with Netty is to use an initial decoding layer to convert a TCP stream into a series of unparsed packet objects, which are defined as consisting of a packet type, length, and buffer containing the actual packet contents. (no byte stuffing is performed). I then feed this packet down the pipeline to one of a few "deserializing processors", each of which handles a set of packet types that are semantically related to each other (for example one such processor might handle connection state messages, while another may handle packets related to a certain aspect of the game itself). As per the design of Netty's MessageToMessageDecoder usage, my decoder is implemented as:

protected void decode (ChannelHandlerContext ctx, UnparsedPacket msg, List<Object> out)
        throws Exception {
    switch (msg.command) {
        case FooPacket.COMMAND_ID:
            FooPacket fooPacket = new FooPacket(msg.frame);
            out.add(fooPacket);
            break;
        case BarPacket.COMMAND_ID:
            BarPacket barPacket = new BarPacket(msg.frame);
            out.add(barPacket);
            break;
    }

}

Once I get a structure decided upon I can easily create this with a code generator.

In addition, I override public boolean acceptInboundMessage(Object) to specify if an unparsed packet should be handled by this decoder.

This is attaining a bit of what appears to be a code-smell for me, so I was looking into alternative solutions, such as:

• Having an array whose (integer keys) map to values of a lambda that takes a RawPacket and returns the parsed packet. This seems like a somewhat weird way to use an array and requires that I manually maintain the definition of that array so that its index-element mapping exactly matches the actual packet type to packet structure mapping already in use.
• Having a map structure that does the same thing as the array. Similar pitfalls, but somewhat more maintainable.

Are these any more novel ways to approach this that I haven't thought of yet?

Answer 1

Ideally, you would override the decode method for each packet type.

But the decode method isn't part of the specific subclass. That is why the switch, or a map that returns the packet-specific factory is justified. It smells because you have to repeat the same code over and over and the method has dependencies on all 40 subclasses. But if the code is generated, I don't see a problem there.

If you want more independence and avoid repeating the same code for each and every subclass, you could map your integer to the packet-specific Class, create an instance from the class and ask the instance to initialize itself from the frame.

You would have something like:

static Class<? extends MyGameNetworkPacket>[] packetClasses;
static {
    packetClasses = new Class[50];
    packetClasses[1] = FooPacket.class;
    packetClasses[2] = BarPacket.class;
    packetClasses[5] = GimmeFivePacket.class;
}

And in the decode method:

Class<? extends MyGameNetworkPacket> packetClass = packetClasses[msg.command];
if( packetClass != null ){
    MyGameNetworkPacket pkt;
    try {
        pkt = packetClass.newInstance();
    } catch( InstantiationException | IllegalAccessException e ){
        throw new RuntimeException("Failed to create instance.", e);
    }
    pkt.fillFrom(msg.frame);
    out.add(pkt);
} else {
    throw new RuntimeException("Invalid command ID: " + msg.command + ".");
}

It doesn't look very elegant because of the try/catch for errors even though they cannot happen if the classes were written properly. But it reduces the dependency on the subclasses to the reference to their class. You could even initialize the array from an external configuration file that contains the class name for each command ID.

Answer 2

What you have described, in one way or another, is exactly how anyone would do it and I don't see any problem with this method.

On a side note: you don't have to use lambda functions in your map/array. A simple interface bound method would be cleaner.