Messaging System Object Oriented Design

Question

I have designed a messaging system below regarding the following assumptions I would be appreciated for the valuable reviews. Especially considering; Multi-threading / SOLID principles / Design patterns

Assumptions:

Messaging System allows users to communicate with each other via messages.

User can be registered to the system via their information name, email etc.

User can add friends and have friends list.

When users login the system they became active.

Every user can have a conversation list for both private and group ones.

Many user can be added to the system.

2 or more users can have conversation and each conversation can be build up from messages.

2 participant conversation can be considered private conversation.

More than 2 conversation can be accepted as group conversation.

User should be friends to send messages to each other.

To being a friend one party should send a request and the other should accept.

There could be waiting requests friendship requests for each user.

MessageSystem.java

package oopdesign.chatServer;

import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class MessageSystem {

    public static void main(String[] args) {

        User user1 = new User("Nesly", "Surname","nesly@gmail.com");
        User user2 = new User("Jack",  "Surname","jack@gmail.com");
        User user3 = new User("Daniel","Surname","daniel@gmail.com");

        UserManagement userManager = UserManagement.getInstance();

        userManager.addUser(user1);
        userManager.addUser(user2);
        userManager.addUser(user3);

        // Get all user list
        HashMap<Integer, User> userAllMap = userManager.getAllUserList();

        System.out.println("All user list");

        for(Map.Entry<Integer, User> userEntry : userAllMap.entrySet()){
            System.out.println("User :" + userEntry.getValue().getName());
        }

        FrienshipRequest frienshipRequest = new FrienshipRequest(user1, user2);
        userManager.acceptFriendshipRequest(frienshipRequest);

        FrienshipRequest frienshipRequest1 = new FrienshipRequest(user2, user3);
        userManager.acceptFriendshipRequest(frienshipRequest1);

        // Get users friends list
        for(Map.Entry<Integer, User> userEntry : userAllMap.entrySet()){
            User user = userEntry.getValue();
            System.out.println("Friends of :" + user.getName());

            HashMap<Integer, User> friendsMap = user.getFriendsList();
            for(Map.Entry<Integer, User> friendsMapEntry : friendsMap.entrySet()) {
                System.out.println(friendsMapEntry.getValue().getName());
            }
        }
        user1.sendMessageToPrivateConversation(user2,"Hi");
        user2.sendMessageToPrivateConversation(user1,"Fine, You ?");

        Conversation conversation = user1.getPrivateConversations(user2);

        List<Message> messageList = conversation.getConversation();

        for(Message message : messageList){
            System.out.println("Message : " + message.getContent() + " / Date :" + message.getDate());
        }
     }
}

Conversation.java

package oopdesign.chatServer;

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

public abstract class Conversation {

    protected int conversationId;
    protected List<Message> conversation = new ArrayList<>();
    protected List<User> participants = new ArrayList<>();

    protected void addParticipant(User user){
        participants.add(user);
    }

    protected void addMessage(Message message){ conversation.add(message); };

    public int getConversationId() {
        return conversationId;
    }

    public List<Message> getConversation() {
        return conversation;
    }

    public List<User> getParticipants() {
        return participants;
    }

}

FrienshipRequest.java

package oopdesign.chatServer;

public class FrienshipRequest {

    private User toUser;
    private User fromUser;

    public FrienshipRequest(User toUser, User fromUser){
        this.toUser = toUser;
        this.fromUser = fromUser;
    }

    public User getToUser() {
        return toUser;
    }

    public User getFromUser() {
        return fromUser;
    }

}

GroupConversation.java

 package oopdesign.chatServer;

 public class GroupConversation extends Conversation {

    public GroupConversation() {
        super();
    }

    @Override
    protected void addParticipant(User user) {
        super.addParticipant(user);
    }

    @Override
    public void addMessage(Message message) { }

}

Message.java

package oopdesign.chatServer;

import java.util.Date;

public class Message {

    private String content;
    private Date date;

    public Message(String content,Date date){
        this.content = content;
        this.date = date;
    }

    public String getContent() {
        return content;
    }

    public Date getDate() {
        return date;
    }

}

PrivateConversation.java

package oopdesign.____.chatServer_ness;

public class PrivateConversation extends Conversation {

    public PrivateConversation(User user1, User user2) {
        participants.add(user1);
        participants.add(user2);
    }

    public User getOtherParticipant(User user){

        if(participants.get(0)==user){
            return participants.get(1);
        }else if(participants.get(1)==user){
            return participants.get(0);
        }
        return null;
    }

}

User.java

package oopdesign.____.chatServer_ness;

import java.util.Date;
import java.util.HashMap;

public class User {

    private int id;
    private String name;
    private String surname;
    private String email;
    private String status;

    private HashMap<Integer, User> waitingRequests = new HashMap<>();
    private HashMap<Integer, User> friendList = new HashMap<>();
    private HashMap<Integer, PrivateConversation> privateConversation = new HashMap<>();
    private HashMap<Integer, GroupConversation> groupConversations = new HashMap<>();

    public User(String name,String surname,String email){
        this.id = name.hashCode();
        this.name = name;
        this.surname = surname;
        this.email = email;
    }

    public void sendMessageToPrivateConversation(User toUser,String content){
        Message message = new Message(content,new Date());
        PrivateConversation conversation = privateConversation.get(toUser.getId());
        if(conversation == null){
            conversation = new PrivateConversation(this, toUser);
        }
        conversation.addMessage(message);
        privateConversation.put(toUser.getId(),conversation);
        toUser.privateConversation.put(this.getId(),conversation);
    }

    public void sendMessageToGroupConversation(GroupConversation conversation,String content){
        Message message = new Message(content,new Date());
        GroupConversation groupConversation = groupConversations.get(conversation);
        if(conversation == null) {
            conversation = new GroupConversation();
        }conversation.addMessage(message);

        groupConversations.put(conversation.getConversationId(),conversation);
    }

    public void addContact(User user) {
        waitingRequests.remove(user.getId());
        friendList.put(user.getId(), user);
    }

    public void rejectContact(User user) {
        waitingRequests.remove(user.getId());
    }

    public void sendFriendshipRequest(User toUser){
        FrienshipRequest fr = new FrienshipRequest(this, toUser);
    }

    public int getId() {
        return id;
    }

    public String getName() {
        return name;
    }

    public String getSurname() {
        return surname;
    }

    public String getEmail() {
        return email;
    }

    public String getStatus() {
        return status;
    }

    public void setStatus(String status) {
        this.status = status;
    }

    public HashMap<Integer, User> getFriendsList() {
        return friendList;
    }

    public HashMap<Integer, User> getWaitingRequests() {
        return waitingRequests;
    }

    public HashMap<Integer, User> getFriendList() {
        return friendList;
    }

    public PrivateConversation getPrivateConversations(User user) {
        return privateConversation.get(user.getId());
    }

    public HashMap<Integer, GroupConversation> getGroupConversations() {
        return groupConversations;
    }
}

UserManagement.java

package oopdesign.____.chatServer_ness;

import java.util.HashMap;

public class UserManagement {

    public static UserManagement userManagement;
    private HashMap<Integer, User> userMap = new HashMap<>();

    public static UserManagement getInstance(){
        if(userManagement == null){
            userManagement = new UserManagement();
        }
        return userManagement;
    }

    public void acceptFriendshipRequest(FrienshipRequest fr){
        User user1 = fr.getFromUser();
        User user2 = fr.getToUser();
        user1.addContact(user2);
        user2.addContact(user1);
    }

    public void rejectFriendshipRequest(FrienshipRequest fr){
        User user1 = fr.getFromUser();
        User user2 = fr.getToUser();
        user2.rejectContact(user1);
    }

    public void addUser(User user){
        userMap.put(user.getId(), user);
    }

    public void removeUser(User user){
        userMap.remove(user.getId());
    }

    public void login(User user){
        user.setStatus("ACTIVE");
    }

    public void logOut(User user){
        user.setStatus("PASSIVE");
    }

    public HashMap<Integer, User> getAllUserList() {
        return userMap;
    }
}

Answer 1

Missing behaviour

public class GroupConversation extends Conversation {
    //...
    @Override
    public void addMessage(Message message) { }
}

GroupConversation's addMessage doesn't delegate and it doesn't do anything else with the message. This seems very suspect.

UserManager

It's generally a bad idea to provide direct access to an internal collection that your class depends on.

public HashMap<Integer, User> getAllUserList() {
    return userMap;
}

By providing this method, it's possible for any client of your userManger to insert/remove items from the userMap. This probably isn't something you want to happen.

I think it's also a little bit odd that you're returning a HashMap. They key in the map seems to be an internal implementation detail, indeed your MessageSystem doesn't use the keys, it only uses the values. It would be better to return something like an UnmodifiableCollection of the map's values.

Friends

The FriendShip request methods don't use any of the class members, (for example to validate that the users being friended both exist within the user manager). The methods could be static.

Your friendship request functions both declare variables user1 and user2. These names hide the context from the request. Why not call them fromUser and toUser. Your friendship system as it stands seems incomplete. You crate a request, but never do anything with it...

public void sendFriendshipRequest(User toUser){
    FrienshipRequest fr = new FrienshipRequest(this, toUser);
}

PrivateConversation

public User getOtherParticipant(User user){
    if(participants.get(0)==user){
        return participants.get(1);
    }else if(participants.get(1)==user){
        return participants.get(0);
    }
    return null;
}

It's unclear why this method would be required (the client can already get all of the participants). If user isn't one of the participants, consider throwing an exception (seems like an invalid state). Alternately, you could return an Optional to indicate the expectation that a user might not be returned. This also looks suspect participants.get(1)==user are you meaning to do a reference comparison here? It seems like two users should be equivalent if their id is the same.

Map Iteration

HashMap<Integer, User> friendsMap = user.getFriendsList();
for(Map.Entry<Integer, User> friendsMapEntry : friendsMap.entrySet()) {
    System.out.println(friendsMapEntry.getValue().getName());
}

If you only care about the values in the map, then you can iterate over them rather than the entries. Which makes the code a more concise/descriptive:

 user.getFriendsList()
     .values()
     .forEach(friend->System.out.println(friend.getName()));

Answer 2

Few possible improvements :

1.User class is highly loaded with collections and methods, it would be better to follow Separation_of_concerns by having user as plain pojo and other responsibility as service like Friendship service, Conversation service.

2.All conversation type can be handled with generic Conversation class defined by you and we can store type information in another collection in order to avoid limitation at storage level.