Encoding and Decoding multiple different types with Netty 5

Question

I am trying to implement a network protocol with multiple different packet types. The problem I am facing is the most "proper" way of implementing this with Netty. I'll post some classes first, and then describe what I want to accomplish.

public class ItemStack {

    public ItemStack(final int item, final int amount) {
        if (item < 0) {
            throw new IllegalArgumentException("item must be non-negative integer: " + item);
        }
        if (amount < 1) {
            throw new IllegalArgumentException("amount must be positive integer: " + amount);
        }
        this.item = item;
        this.amount = amount;
    }

    public int getItem() {
        return item;
    }

    public int getAmount() {
        return amount;
    }

    private final int item;
    private final int amount;
}

public class ChatMessage {

    public ChatMessage(final String playerName, final String message) {
        if (playerName == null) {
            throw new IllegalArgumentException("playerName must not be null");
        }
        if (message == null) {
            throw new IllegalArgumentException("message must not be null");
        }
        this.playerName = playerName;
        this.message = message;
    }

    public String getPlayerName() {
        return playerName;
    }

    public String getMessage() {
        return message;
    }

    private final int playerName;
    private final int message;
}

Now, all POJO that are transmitted across the network will have a packet identifier. This will be a 1-byte code that will let the decoder know what type of packet it is, and how to decode it.

What would be the most appropriate way of handling this case? Would it be better (read more conventional) to have one PacketDecoder class that extends the ByteToMessageDecoder which reads one byte, determines the type, and then in that same class, decode the packet with the appropriate method like so:

public class PacketDecoder extends ByteToMessageDecoder {

    protected void decode(
        final ChannelHandlerContext context, final ByteBuf buf, List<Object> objects) throws Exception {
        if (buf.readableBytes < 1) {
            return;
        }
        final int opcode = buf.readByte() & 0xff;
        final Packet packet = decodePacket(opcode);
        objects.add(packet);
    }

    private Packet decodePacket(final int opcode, final ByteBuf buf) {
        if (buf == null) {
            throw new IllegalArgumentException("buf must not be null");
        }
        Packet packet = null;
        switch (opcode) {
            case 0:
                packet = decodeItemStack(buf);
                break;

            case 1:
                packet = decodeChatMessage(buf);
                break;
            // ...
        }
        return packet;
    }
}

Or would it be better to just add every type of decoder to the pipeline?

Answer 1

I did this in my own program and I used a single decoder because it was more straight forward thing to do. The only reason I could see for wanting multiple decoders would be if need to extend or dynamically changing the protocol your server understands. For example, maybe certain aspects of your server are free and others are paid for extensions that license key turns on then I could see this architecture being important. Or you load extensions to the protocol dynamically maybe. I think you need a real reason to segment decoding into several decoders besides it being architecturally pure.

In this case you can add multiple decoders to the pipeline, but each decoder needs to play nice and forward packets not meant for it along to the next decoder in the pipeline. You also have to be careful not to pull bytes off that down stream decoders might need.

Here is what I wouldn't do. A decoder per message architecture. That will be cumbersome to write and maintain. Since there is overhead with each decoder written to play nice and forward packets I wouldn't go through that exercise each time I write a decoder. You could overcome that with a nice base class to extend, but why go through all that hassle when you can just parse the first byte and do a simple if ladder?