How to avoid using instanceof / how to use polymorphism here

Question

I'm developing my first network app and I have found some troubles I am not sure how to solve. I have the following hierarchie for the paquets

interface Packet {}
class NewClientPacket implements Packet {}
class DisconnectPacket implements Packet {}
class DataPacket implements Packet {}
...

Now, the server needs to handle any of this packets which may be sent by a client and do different things for each packet. One the first things I would write is:

Packet packet = (Packet) myStream.readObject();
if (packet instanceof NewClientPacket) { 
    ...
} else if (packet instanceof DisconnectPacket { 
    ...
} else if (packet instanceof DataPacket) {
    ...
} 
...

but I dont like this at all (it uses instanceof, scales bad when adding a lot of new Packet subclasses, and it's very verbose...)

I find that usually when I have to use instanceof, I can avoid it by using polymorphism, so I though about changing the Packet interface to

interface Packet {
    void handle(PacketHandler handler);
}

and then I can just do

Packet packet = (Packet) myStream.readObject();
packet.handle(this);

but I don't know if this is good way to solve the problem. Could you suggest others, or comment about mine?

Answer 1

I this context, the only object that is and should be aware of its actual type is the Packet instance. You would therefore write a Handler that is given to the Packet and dispatched from there. This would look something like the following:

interface Handler {
  void handle(NewClientPacket packet);
  void handle(DisconnectPacket packet);
  void handle(DataPacket packet);
}

interface Packet {
  void dispatch(Handler handler)
}

class NewClientPacket implements Packet {
  @Override
  public void dispatch(Handler handler) { 
    handler.handle(this);
  }
}

class DisconnectPacket implements Packet {
  @Override
  public void dispatch(Handler handler) { 
    handler.handle(this);
  }
}

class DataPacket implements Packet {
  @Override
  public void dispatch(Handler handler) { 
    handler.handle(this);
  }
}

All implementations will invoke the correct handle method by their type. This approach is named the Visitor Pattern. In reality, you should choose less generic names for the methods to make your code better readable. Often, domain-specific names are used.

Answer 2

This is a Visitor Pattern

First you need a Visitor

public class PacketVisitor {

    void visit(NewClientPacket packet);
    void visit(DisconnectPacket packet);
    void visit(DataPacket packet);

}

Then you need to add a method to your interface Packet:

interface Packet {

    void accept(PacketVisitor visitor);
}

Now in each Packet you need to implement this method:

public class NewClientPacket implements Packet {

    @Override
    public void accept(PacketVisitor visitor) {
        visitor.visit(this);
    }

}

Finally in your service code:

final PacketVisitor visitor = new PacketVisitor() {
   //implementation...
}
final Packet packet = (Packet) myStream.readObject();
packet.accept(visitor);

What will happen is that the relevant visit method of the PacketVisitor will be called as the instance of Packet calls visit on the PacketVisitor.

Answer 3

While the visitor pattern can solve this for you, you need to watch your dependencies. You probably want to split everything into 3 separate packages:

• client only
• server only
• shared classes

Client / server will have a dependency on the shared classes, but no other dependencies between those 3 must exist.

All packets would obviously go into the shared classes, but their respective client / server side handler would be private to the client / server respectively. So packet can't have any dependency on client or server classes; this makes it impossible to have the packet supply the handler/dispatch (packet can't possibly know its handler in this scenario).

To implement the visitor pattern you will have an abstract visitor in shared classes and client / server actually implement it. That way the packets can have a dependency on the abstract visitor:

abstract class Dispatcher {
    public abstract void handleX(X x);

    public abstract void handleX(Y y);
}

class X extends Packet {
    public void dispatch(Dispatcher d) {
         d.handleX(this);
    }
}

Personally I'm not the biggest fan of this pattern as it forces you to add mehods to the visitor whenever you add a new packet, but thats the way you have it nice and compile time type checked. For a small to moderate number of packets this is probably the nicest solution you will find.

In practice it may be more convenient to just sacrifice compile time safety and figure out the proper packet handler using a naming convention and reflection.