3
\$\begingroup\$

I'm writing this simple chat room in Java, and I wish to refactor a couple of things, like the following switch statement that should be replaced with polymorphism or at least not rely on another class enum.

Message Object:

public class Message implements Serializable { public enum MessageTypes {PUBLIC_MESSAGE,PTIVATE_MASSAGE,USER_DUPLICATE,USER_LOGOUT,SERVER_LOGOUT,USERSLIST}; private String dest; private String src; private String messageContent; private MessageTypes type; public Message(String source) { src = source; } public Message(String source,String message) { this(source); messageContent = message; type = MessageTypes.PUBLIC_MESSAGE; } public Message(String source,String destination,String message) { this(source); messageContent = message; dest= destination; type = MessageTypes.PTIVATE_MASSAGE; } public String getDest() { return dest; } public String getSrc() { return src; } public String getMessageContent() { return messageContent; } public MessageTypes getType() { return type; } private void setType(MessageTypes typeToSet) { type = typeToSet; } public static Message getServerLogoutMessage() { Message logoutMessage = new Message("Server"); logoutMessage.setType(MessageTypes.SERVER_LOGOUT); return logoutMessage; } public static Message getServerLogoutMessage(String src) { Message logoutMessage = new Message(src); logoutMessage.setType(MessageTypes.SERVER_LOGOUT); return logoutMessage; } public static Message getUserLogoutMessage(String source) { Message logoutMessage = new Message(source); logoutMessage.setType(MessageTypes.USER_LOGOUT); return logoutMessage; } public static Message getUsersListMessage(String source) { Message usersMessage = new Message(source); usersMessage.setType(MessageTypes.USERSLIST); return usersMessage; } public static Message getDuplicateMessage() { Message msg = new Message("Server"); msg.setType(MessageTypes.USER_DUPLICATE); return msg; } @Override public String toString() { return "Source : " + getSrc() + "\n" + "Dest : " + getDest() + "\n" + "Type : " + getType() + "\n" + "Content : " + getMessageContent() + "\n"; } }

Function at the server class:

public synchronized void handelMessage(Message msg) { ClientListenerWriterPair p; String src = msg.getSrc(); String content = msg.getMessageContent(); String toWrite; switch (msg.getType()) { case PTIVATE_MASSAGE: sendPrivateMessage(msg); break; case PUBLIC_MESSAGE: toWrite = String.format("%s : %s",src,content); control.writeTextToGui(toWrite); spreadTheMessage(msg); break; case USER_LOGOUT: p = nameToClientHash.remove(msg.getSrc()); p.getListener().close(); p.getWriter().close(); p.getWriter().setMsg(Message.getUserLogoutMessage("Server")); try { p.getWriter().join(); } catch (InterruptedException e) { // TODO Auto-generated catch block e.printStackTrace(); } p.close(); break; case SERVER_LOGOUT: p = nameToClientHash.remove(msg.getSrc()); p.getListener().close(); p.close(); break; case USERSLIST: p = nameToClientHash.get(msg.getSrc()); final Set<String> userSet = nameToClientHash.keySet(); break; } }
\$\endgroup\$

1 Answer 1

Reset to default
1
\$\begingroup\$

Handling messages

[...] I wish to refactor a couple of things, like the following switch statement that should be replaced with polymorphism or at least not rely on other class enum.

You could create MessageHandler classes that implement the actions in the current case statements, and register them in a MessageHandlerRegistry, that has a Map<MessageType, MessageHandler>. And then, as you process incoming Message instances, you could get the MessageHandler from the MessageHandlerRegistry to perform the appropriate actions. Something along the lines of:

 interface MessageHandler { void handle(); } interface MessageHandlerRegistry { void register(MessageType type, MessageHandler handler); MessageHandler get(MessageType type); } // ... void processMessage(Message message) { messageHandlerRegistry.get(message.type()).handle(); }

Creating Message instances

In Message you have several static methods that create messages, for example:

public static Message getServerLogoutMessage() { Message logoutMessage = new Message("Server"); logoutMessage.setType(MessageTypes.SERVER_LOGOUT); return logoutMessage; }

Instead of naming these getSomething it would be better to name them newSomething, to indicate new instance creation.

But a bigger problem is that creating Message instances is a bit tedious, due to the setType call that forces you to write 3 lines per message type.

It would be better to use the builder pattern, so that you can create messages more fluently like this:

public static Message newServerLogoutMessage() { return builder().source("Server").type(MessageType.SERVER_LOGOUT).build(); }

And I would actually keep the Message class free from all this stuff, and move these helper factory methods to a Messages class, which is a common pattern, seen for example in the Collections class of the JDK.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.