2
\$\begingroup\$

The following is my little chat room system/structure. I've tried to follow the definition/intent of mediator pattern by GoF:

Allows loose coupling by encapsulating the way disparate sets of objects interact and communicate with each other. Allows for the actions of each object set to vary independently of one another.

But I still have a lot of questions about mediator pattern. This system is my trying and I need any advice about any aspect(pros/cons, alternatives, ...) of my design. All will be deeply appreciated.


These are questions you would ask after review my program:

Q: Why not make the RoomMate an abstract class instead of an interface, so it can keep an reference to the ChatRoom the roommate is currently in?

A: I think an interface would be more useful since a class can implement more than one interfaces in Java. (But all examples I've found from the Internet make Colleague an abstract class and make it keep a reference of Mediator...)

Q: Why do you repeat yourself that Amy, Andy, and Lily all treat the message the same way?

A: Maybe I should consider the strategy pattern. But I just want to focus on the Mediator.

Q: Ok, each concrete mediator knows how many roommates it has. But, reversely, if Lily wants to know how many rooms she has joined?

A: Actually I don't know how to solve this. (But I think maybe that's Lily's responsibility to remember what she's doing)


Role:

  • ChatRoom --> Mediator
  • RoomMate --> Colleague

UML:

Mediator by @N1ng only for CodeReview.StackExchange

Code:

ChatRoom.java

public abstract class ChatRoom
{
    public abstract void accommodate(RoomMate roomMate);
    public abstract void fairwell(RoomMate roomMate);
    public abstract void sendMessage(RoomMate roomMate, String message);
    public abstract void printRoomMates();
    public abstract boolean hasRoomMate(RoomMate roomMate);
}

Lounge.java

public class Lounge extends ChatRoom
{
    private ArrayList<RoomMate> roomMates;
    private String name;

    public Lounge() {
        this.roomMates = new ArrayList<RoomMate>();
        this.name = "Lounge";
    }

    public void accommodate(RoomMate roomMate) {
        getRoomMates().add(roomMate);
    }

    public void fairwell(RoomMate roomMate) {
        if (hasRoomMate(roomMate)) {
            roomMates.remove(roomMate);
            System.out.format("[%s] Fairwell! %s! we miss you!\n",
                getName(),
                roomMate.getClass().getSimpleName()
            );
        } else {
            System.out.format("[%s] You can't leave twice! %s\n",
                getName(),
                roomMate.getClass().getSimpleName()
            );
        }
    }

    public void sendMessage(RoomMate roomMate, String message) {
        for (RoomMate mate: getRoomMates()) {
            if (mate != roomMate) {
                mate.receiveMessage(roomMate, message);
            }
        }
    }

    public void printRoomMates() {
        if (!getRoomMates().isEmpty()) {
            System.out.format("[%s] There are:\n", getName());
            for (RoomMate mate: getRoomMates()) {
                System.out.format("  %s\n", mate.getClass().getSimpleName());
            }
        } else {
            System.out.format("[%s] This room is empty now...\n", getName());
        }
    }
    public boolean hasRoomMate(RoomMate roomMate) {
        if (getRoomMates().indexOf(roomMate) >= 0) {
            return true;
        }
        return false;
    }

    // getter & setter
    public String getName() {
        return this.name;
    }

    public ArrayList<RoomMate> getRoomMates() {
        return this.roomMates;
    }
}

RoomMate.java

public interface RoomMate {
    public abstract void receiveMessage(RoomMate roomMate, String message);
}

Amy.java

public class Amy implements RoomMate {

    @Override
    public void receiveMessage(RoomMate roomMate, String message) {
        System.out.format("[%s][receive_message] %s: %s\n",
            this.getClass().getSimpleName(),
            roomMate.getClass().getSimpleName(),
            message
        );
    }
}

Lily.java

public class Lily implements RoomMate {

    @Override
    public void receiveMessage(RoomMate roomMate, String message) {
        System.out.format("[%s][receive_message] %s: %s\n",
            this.getClass().getSimpleName(),
            roomMate.getClass().getSimpleName(),
            message
        );
    }
}

Andy.java

public class Andy implements RoomMate {

    @Override
    public void receiveMessage(RoomMate roomMate, String message) {
        System.out.format("[%s][receive_message] %s: %s\n",
            this.getClass().getSimpleName(),
            roomMate.getClass().getSimpleName(),
            message
        );
    }
}

Main.java

public class Main
{
    public static void main(String[] args) {
        RoomMate andy = new Andy();
        RoomMate amy = new Amy();
        RoomMate lily = new Lily();
        ChatRoom lounge = new Lounge();

        lounge.accommodate(andy);
        lounge.printRoomMates();

        lounge.accommodate(amy);
        lounge.printRoomMates();

        lounge.accommodate(lily);
        lounge.printRoomMates();

        lounge.sendMessage(andy, "Hi everyone :) !");
        lounge.sendMessage(lily, "Hello, nice to meet you~ :\")");
        lounge.sendMessage(amy, "It's my pleasure to join you! v_v");

        lounge.hasRoomMate(andy);
        lounge.fairwell(andy);
        lounge.fairwell(andy);
        lounge.fairwell(amy);
        lounge.fairwell(lily);

        lounge.printRoomMates();
    }
}

The result:

[Lounge] There are:
  Andy
[Lounge] There are:
  Andy
  Amy
[Lounge] There are:
  Andy
  Amy
  Lily
[Amy][receive_message] Andy: Hi everyone :) !
[Lily][receive_message] Andy: Hi everyone :) !
[Andy][receive_message] Lily: Hello, nice to meet you~ :")
[Amy][receive_message] Lily: Hello, nice to meet you~ :")
[Andy][receive_message] Amy: It's my pleasure to join you! v_v
[Lily][receive_message] Amy: It's my pleasure to join you! v_v
[Lounge] Fairwell! Andy! we miss you!
[Lounge] You can't leave twice! Andy
[Lounge] Fairwell! Amy! we miss you!
[Lounge] Fairwell! Lily! we miss you!
[Lounge] This room is empty now...
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Nitpicks

    public void fairwell(RoomMate roomMate) {
        if (hasRoomMate(roomMate)) {
            roomMates.remove(roomMate);
            System.out.format("[%s] Fairwell! %s! we miss you!\n",

Should be

    public void farewell(RoomMate roomMate) {
        if (hasRoomMate(roomMate)) {
            roomMates.remove(roomMate);
            System.out.format("[%s] Farewell, %s! We'll miss you!\n",

"Fairwell" is not a word. And ! ends a sentence, so the next word should be capitalized. We will miss you in the future when you're not here is more important than the current instant of emotion.

Interfaces over implementations

    private ArrayList<RoomMate> roomMates;

Consider

    private List<RoomMate> roomMates = new ArrayList<>();

Now, if you want to change the implementation, you just have to change this line. And if you try to do something specific to this implementation but not in the interface, it will tell you.

Since you never do anything but make a blank list of roommates, you might as well do the initialization at declaration time. You could do this with the name as well and get rid of the current constructor entirely. Or do something like

    public Lounge() {
        this("Lounge");
    }

    public Lounge(String name) {
        this.name = name;
    }

This allows you to set the name and provides a default if you don't do so.

It's worth noting that we only have to use this. here in the second one because the method parameter has the same name as the object field. If they had different names, then the this. is unnecessary. Some prefer it for clarity though.

Early exit

    public void printRoomMates() {
        if (!getRoomMates().isEmpty()) {
            System.out.format("[%s] There are:\n", getName());
            for (RoomMate mate: getRoomMates()) {
                System.out.format("  %s\n", mate.getClass().getSimpleName());
            }
        } else {
            System.out.format("[%s] This room is empty now...\n", getName());
        }
    }

I'd find this easier to follow as

    public void printRoomMates() {
        if (getRoomMates().isEmpty()) {
            System.out.format("[%s] This room is empty now...\n", getName());
            return;
        }

        System.out.format("[%s] There are:\n", getName());
        for (RoomMate mate: getRoomMates()) {
            System.out.format("  %s\n", mate.getClass().getSimpleName());
        }
    }

This gets rid of the if not else structure. Now we check and immediately return if true. This makes the check implicitly untrue for the rest of the method. We dispose of the uncommon possibility before getting to the meat of the method.

This also saves us some indent on the bulk of the method.

Avoid reinventing the wheel

    public boolean hasRoomMate(RoomMate roomMate) {
        if (getRoomMates().indexOf(roomMate) >= 0) {
            return true;
        }
        return false;
    }

This can be simplified to

    public boolean hasRoomMate(RoomMate roomMate) {
        return getRoomMates().contains(roomMate);
    }

The if (isTrue()) return true; else return false; pattern is a code smell. You can always simplify that to return isTrue();.

The contains method answers the question of whether roomMate is in roomMates. Consider changing roomMates to a Set instead of a List to make this more efficient.

Abstract classes

There is an argument that people should never use abstract classes in Java because they restrict to single inheritance and because they make it unclear where the behavior is defined. That said, making RoomMate an abstract class would mean that you'd only have to write receiveMessage once. As it is, you have to write it once for each implementing class.

Classes vs. objects

Inheritance examples do this a lot. They make a new class (type) for something that is really just a new instance. Andy, Lily, and Amy should not be classes in my opinion. Instead, make RoomMate a regular class and do something like

RoomMate amy = new RoomMate("Amy"));

Now the name is a property of the object, not a class name.

So to register a new user, you just create a new instance of the class. In the original, user registration required code writing. Not something that scales to tens of thousands of users.

roomMate.getClass().getSimpleName()

We can now say something like

roomMate.getName()

Much simpler all around.

Remember that design patterns are meant to help you. If you are doing things just to fit the pattern, then you probably selected the wrong design pattern.

You never actually use the Mediator pattern's strength, the ability to implement receiveMessage in different ways. You loosely coupled something that actually has universal behavior. It would actually make more sense to make ChatRoom a Mediator. I can think of a number of ways to implement farewell. So we could have a SpanishChatRoom, EnglishChatRoom, LeetChatRoom, etc. Each could have its own implementations of farewell, accommodate, and printRoomMates to fit the appropriate dialect. Of course, I would probably then tell you that the differences between those were just data and suggest that you move the translations out.

What CS.SE says about the Mediator pattern.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.