Suitable Design Pattern for a Simple Chat Application.

Question

Hi.

I am designing a simple chat application (just for the kick of it). I have been wondering of a simple design for that chat application. To give you overview.. here are the rules:

1. Anonymous user enter chat just with a nickname. (User id) is presumably assigned by system in background.
2. They can join in (subscribe to) a chat conversation. And he'll see the chat-text from other users appearing on the designated area.
3. They can reply to a particular conversation and everyone else should see that.

THATS IT! (See I told you it was a simple chat application). So, my intention is not really the application; but the design pattern and objects used in it.

Now here is how I have designed it. (I am coding in java.. in case that really matters)

1. User Object - Two attributes id and nickname
2. Message Object - A simple Message interface and implementation (for now) as a SimpleMessage, with a String as the attribute to contain the message.
3. Chat Window Object - Basically composition of User and Messages. As it has One User Object and List of Messages.
4. Chat Session - Again composition. Basically it'll have a List of Chat Windows. Each chat window registers to a chat session. Chat session is responsible to notify all chat windows when a new message appears. (Observer pattern anyone?)

Alright.. so now I have implemented observer pattern by making ChatWindow implement "ChatListener" patter which has method called "notify(Message)". So ChatSession notifies every registered ChatWindow.

Now here are few things I want to clarify/want your opinion on. 1. I need to have de-register method also for all chat windows, in case a chat window is closed and doesn't want to get any more notifications. That potentially means, that either I should have a "Static" central registration manager which has only one instance and then any chat window should be able to de-register itself by providing a "chat session" id. For that reason, each chat session should have an id. (Included hereon). OR I could maintain an instance of ChatSession in Chat Window too, to always have an instance ready. (I hate singletons as I think they go against oops). Another way would be to not have de-registration control of chat window, with chat window, instead the notification of window closure should come directly to ChatSession and it should do, what it should do!

1. Does this design makes sense at all? If you'd say it's a piece of CRAP and give me a yet better approach; you'll definitely get a BIG THANK YOU from me. Apart from observer pattern what all patterns could be used here to simplify it more or make it better. Also.. any weak points of this design, if it's appropriate but can be improved.

2. Also when a user types a new message in his own chat window; it needs to be propogated to all chat windows, which is what chat session does, but at the same time; does that mean that.. chat session needs to get a message with "chat window id" and message? And then it propogates it to all windows, including the one which is the owner of the message? What is a better way to handle this. I mean, a way where window lets the chat session know of the message and then chat session to all other windows. (I am thinking it'll need some if's... don't like them either)

   Anyway... do let me know your comments. Also please rem. working application is not the intent, I am looking for a good discussion, good Design pattern practices and usage.

Complete code below, if it gives you a high... Feel free to tear it apart and bring up issues related to almost any semantics.

package com.oo.chat;

public class User {

    private Long userId;
    private String nickname;

    public User(Long userId, String nickname) {
     this.userId = userId;
     this.nickname = nickname;
    }

    public void setUserId(Long userId) {
     this.userId = userId;
    }

    public void setNickname(String nickname) {
     this.nickname = nickname;
    }

    public Long getUserId() {
     return userId;
    }

    public String getNickname() {
     return nickname;
    }

    public boolean equals(Object objectToCompare) {
     if (!(objectToCompare instanceof User)) {
      return false;
     }
     User incoming = (User) objectToCompare;
     if (incoming.getNickname() != null && incoming.getUserId() != null) {
      if (incoming.getNickname().equalsIgnoreCase(this.nickname)
        && incoming.getUserId().equals(this.userId))
       return true;
     }
     return false;
    }
}


package com.oo.chat;

public interface Message {

    public String getValue();

    public void setValue(String value);

}

package com.oo.chat;

public class SimpleMessage implements Message {

    private String value;

    public SimpleMessage() {

    }

    public SimpleMessage(String value) {
     this.value = value;
    }

    public String getValue() {
     return value;
    }

    public void setValue(String value) {
     this.value = value;
    }
}

package com.oo.chat;

public interface ChatListener {

    public void notify(Message newMessage);

}

package com.oo.chat;

import java.util.ArrayList;
import java.util.List;

public class ChatWindow implements ChatListener {

    private User user;
    private List<Message> messageList;
    private Long id;

    public User getUser() {
     return user;
    }

    public List<Message> getMessageList() {
     return messageList;
    }

    public void setUser(User user) {
     this.user = user;
    }

    public void setMessageList(List<Message> messageList) {
     this.messageList = messageList;
    }

    public void addMessageToList(Message newMessage) {
     if (this.messageList == null) {
      this.messageList = new ArrayList<Message>();
     }
     this.messageList.add(newMessage);
    }

    public void notify(Message newMessage) {
     addMessageToList(newMessage);
    }

    public Long getId() {
     return id;
    }

    public void setId(Long id) {
     this.id = id;
    }
}

package com.oo.chat;

import java.util.ArrayList;
import java.util.List;

public class ChatSession {

    private List<ChatListener> registeredChatListeners;

    public void register(ChatWindow chatWindow) {
     if (registeredChatListeners == null)
      registeredChatListeners = new ArrayList<ChatListener>();
     registeredChatListeners.add(chatWindow);
    }

    public List<ChatListener> getRegisteredChatListeners() {
     return registeredChatListeners;
    }

    public void setRegisteredChatWindows(
      List<ChatListener> registeredChatListeners) {
     this.registeredChatListeners = registeredChatListeners;
    }

    public void incomingMessage(Long chatListenerId, Message message) {
     publish(message);
    }

    protected void publish(Message messageToPublish) {
     if (registeredChatListeners != null) {
      for (ChatListener eachListener : registeredChatListeners) {
       eachListener.notify(messageToPublish);
      }
     }
    }
}

Thanks to all contributers in advance. Cheers

Answer 1

The basic design looks sound to me. Obviously to complete this, you would nee to add a lot more features. The current design keeps all messages in memory indefinitely, but at some point you are going to need code for purging old messages.

The few significant design issues that I do see are:

1. The message interface doesn't link back the the sender of the message - Most chats show who said what, and this will be difficult without a User field in the message.
2. The message interface doesn't have a time property. This will make purging old messages more difficult.

Answer 2

I would suggest looking into messaging frameworks instead of using Observer pattern.

Take a look at this simple implementation which will be sufficient for your toy project - eventbus. Alternatively you could go with full blown JMS implementation like ActiveMQ.

Basically it allows you to define a common bus to which you can register and unregister participants, and also send messages that all of the participants will see. The big advantage over observer pattern is the very low coupling between the participants - you don't register with each object to get his messages - you just register once with the bus. In addition you get asynchronous processing - let's say you have 1000 chat sessions - if you use observer it means that for each message to complete it will take to update 1000 sessions. With message framework message send is very quick, and notifying all 1000 sessions is done in the background.

Answer 3

Just looking at the User object, why does equality depend on id and nickname ? That seems a little counter intuitive to me. I would expect if you have an id, then that is the identification of the object, and hence what you'd use in an equality condition.

I see also that you have a setter for the user id. So do you really want to change user id ? I see that you can change the nickname, which makes sense. But I would expect the id to remain constant.

Note also that since you're overriding equals(), you should also override hashCode().

Now, if hashCode() and equals() rely on immutable fields (e.g. the id), then the results of hashCode() won't change, and if you put a User in a hashed collection (e.g. HashMap) then you're not going to lose it later (which is very confusing)!

Finally (!) I would protect the constructor and setters against null nicknames (make them throw IllegalArgumentExceptions) and then code like equals() doesn't have to worry about a null nickname (unless 'null' has a meaning for the nickname). I would do the same for id, since you have this as a Long (object). But couldn't this be a primitive long ?