1

In a server code, I have a ProtocolHandler class which reads from the socket, finds out which type of packet it's dealing with and dispatches it to the client. I'm trying to have the following architecture:

public interface Packet { //... } public class ClientInformations implements Packet { //... } public class ProtocolHandler { //.... public void bytesReceived(byte[] bytes) { //... Determine the type of the packet Packet packet = determineTypeOfPacketAndRead(bytes); // Here I already have the packet object built, // like ClientInformations@1a1a1a1a[...] client.packetReceived(packet); } //... } public class Client { //... public void packetReceived(Packet pkt) { System.out.println("Unimplemented packet received."); } public void packetReceived(ClientInformations ci) { } //... }

(I don't know how to explain it easily with words). The problem is, I really thought packetReceived(ClientInformations) would be called, but it's not. It's calling the more general packetReceived(Packet) method. Am I wrong? How can I still use the same architecture then?

--EDIT

Now I understand why that happens. The problem is, i have other packet classes, such as Movement, Sync, Spawn. I wanted to ease adding new packets by just having to add a new method to the Client class. Isn't there another way of doing that then? I mean, a way to automatically analyse the type of the packet object at runtime and doing the most specific method call?

Improve this question
2
  • 1
    Can you add all relevant code of bytesReceived? I cannot know of which type packet is, even though you have added System.out.println(packet), I'd rather not focus on that. Commented Dec 22, 2013 at 18:13
  • 1
    I assume that determineTypeOfPacketAndRead is declared to returns Packet. Because of that compiler cant link it to packetReceived(ClientInformations ci). You can do it manually by casting result to ClientInformations after checking its type with instanceof. Commented Dec 22, 2013 at 18:15

6 Answers 6

Reset to default
5

I assume you are having Packet packet = determineTYpeOfPacketAndRead(bytes), as there is hardly an other way the code would else compile.

So the reference type is Packet (at compile-time), even though the object type is ClientInformations (at runtime).

And as specified in the Java Language Specification Chapter 15 onto Method Overloading.

The most specific method is chosen at compile-time; its descriptor determines what method is actually executed at run-time.

UPDATE: Your code currently works, because you have no ambiguity in your code, even though it does not behave the way you want. However you will not be able to extend this code, as it will lead to issues, let me illustrate:

An earlier suggestion of mine was the following:

public class Client { //... public void unknownPacketReceived(Packet pkt) { System.out.println("Unimplemented packet received."); } public void packetReceived(ClientInformations ci) { } //... }

However this cannot work as a ClientInformations object is something more sophisticated as a simple Packet object/interface, it will give the following compile error:

incompatible types: Packet cannot be converted to ClientInformations

This happens as the reference type is still only Packet, while you want to lift it to being a ClientInformations.

It could work however if you would call it as:

client.receivedPacket((ClientInformations)packet)

This can do no harm in your case as on runtime they are of the same type, so nothing is lost in the conversion, this would need to be accompanied with an instanceof tree, which is why you are back at step zero now.

This program is not just going to work, I'd suggest to change your design if you want it to be simple, continuing this path may be an option if you are working on a legacy application and cannot just change things, but it won't be easy.

Improve this answer
Sign up to request clarification or add additional context in comments.

8 Comments

More information here.
Ok, I understand now, but does this means I can't do this kind of thing? I was hoping adding new packet types would be as simple as adding another method to the Client class...
That I cannot answer, (maybe someone else can), you surely need to have something like packetReceived() and unknownPacketReceived() though.
Should I open a new question on that topic?
@wingleader I am not sure if that is an option in your case but maybe consider overwriting rather then overloading. What I mean is trying to add some method to your interface and implement it differently in each of ClientInformationsX class. This way when you use myPacket.mySpecialMethod(client) you will get results based on actual type of ClientInformationsX.
|
1

Java is a single-dispatch language, so the trick is to learn to leverage that to the best effect possible.

In your case, you do not need to dispatch on the Client type, but you do need it against the Packet type. Let this requirement guide your design: instead of adding method overloads to Client, have Packet contain the handling code in a method like

void handle(Client c)

and make everything from the client available to the Packet. This will give you the chance to provide different handling in each specialization of Packet.

Unfortunately, the type dispatched on must the type owning the method in question, which may hurt your need to separate concerns. This could be accomplished with even more architecture, but I hope this guideline will help you on your way.

Improve this answer

Comments

1

The thing is, if I understand you correctly, the object you are passing is both a Packet and a ClientInformations, so neither of the methods really has any precedence over the others.

Just give the other method a different name and your problem will be solved.

Improve this answer

Comments

1

You have to do it as follows:

public void bytesReceived(byte[] bytes) { //... Determine the type of the packet packet = determineTypeOfPacketAndRead(bytes); // Here I already have the packet object built, // like ClientInformations@1a1a1a1a[...] if (packet instanceof ClientInformations) { ClientInformations ci = (ClientInformations)packet; client.packetReceived(ci); // this way, it's clear which method you want to call } }

However, I prefer Tim B's suggestion to change the method name. I suggest clientInformationsPacketReceived().

Improve this answer

Comments

1

The answer to why has already been shown in the other answers. A solution (as far as I can tell) would require a change in your thought process.

Right now you're trying to put the logic to handle a packet in some sort of general class that handles this for every type of packet. This is a fairly common anti-pattern: creating 'manager' classes that will do the work for the specific objects.

An easy solution would be to simply put this logic in your classes themselves.

When you have the following situation

interface Packet { void packetReceived(); } class ClientInformations implements Packet { @Override public void packetReceived() { System.out.println("I am ClientInformations"); } }

You can just call packetReceived on your variable packet and this will do the work for you.

Improve this answer

10 Comments

I'm a bit worried about the Single Responsibility Principle and the separation of data and logic (Packet implementors where supposed to just implement reading and writing the packet to/from the stream), but I'm leaning towards this approach now. It sure is different from the usual architecture I see, but it seems smart. I was thinking in adding another layer of separation from data and logic between packet data and action, do you believe that's necessary?
@wingleader It depends on who is working on the various packet implementations. On big projects you want maximum seperation, however on small projects you can get away with these things easily. And even in big projects it shouldn't be an issue if everyone follows your guidelines. Though in all honesty a packet itself has nothing to do with it being received or not, that is the Clients call and thus code.
Can you clarify what layer you'd place in between? Right now I'm a little torn whether or not this would be an acceptable architecture with regards to the SRP. I don't believe it violates the separation of data and logic since a Packet in itself doesn't contain any data-source-access and classes should definitely perform logic on themselves when possible, otherwise they're just variable wrappers.
But what if the handling code involves OP's Client class more than the Packet class? Handing over the code to the Packet hierarchy, and having to pass the Client instance to packetReceived, intimately couples these two types. Depending on the rest of the system, this may be entirely inappropriate.
OP, this is another important consideration: does Client need to know about each packet type that can be received? My guess: yes. In that case there is not much to be gained by relying on polymorphism/dynamic dispatch to vary the handling according to packet type. You could instead have a Map<Class<? extends Packet>, PacketHandlingStrategy> inside Client. PacketHandlingStrategy would be implemented as inner classes inside Client to facilitate acces to its internals. Note that in Java 8, the code for this would look much cleaner.
|
0

You can typecast your object reference to call the packetReceived(ClientInformations) instead of packetReceived(Packet).

You can call like this client.packetReceived((ClientInformations)packet);

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.