current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

I have implemented the following scenario for the Mediator pattern sample:

MailingListMediator: Manages the subscription and send the mail back and forth.

  • JavaMailingList
  • SQLMailingList

(Colleagues)MailUsers: They can login, subscribe to the mailing list, send mails to mailing list.

  • GmailUsers
  • YahooUsers

My implementation looks like this:

MailingListMediator 

public abstract class MailingListMediator {

    protected List<MailUser> mailingList;

    public MailingListMediator() {
        this.mailingList = new ArrayList<MailUser>();
    }

    public void sendMail(String message){
        for(MailUser mUser:mailingList){
            System.out.println("Mail message: '"+message+"' sent to "+ mUser.getMailId());
        }
    }
    public abstract String subscribe(MailUser mailUser);
}

MailUser

public abstract class MailUser {
    public abstract void login(String mailID);
    public abstract void sendMailToMailingList(MailingListMediator mailingList, String message);
    public abstract String getMailId();
}

GmailUser 

public class GmailUser extends MailUser {
    String mailID;

    @Override
    public void login(String mailID) {
        this.mailID = mailID;
    }

    @Override
    public void sendMailToMailingList(MailingListMediator mailingList, String message)     {
        mailingList.sendMail(message);
    }

    @Override
    public String getMailId() {
        if(mailID != null){
            return mailID;
        }else{
            return "Login First";
        }    
    }
}

YahooUser 

public class YahooUser extends MailUser {
    String mailID;

    @Override
    public void login(String mailID) {
        this.mailID = mailID;
    }

    @Override
    public void sendMailToMailingList(MailingListMediator mailingList, String message) {
        mailingList.sendMail(message);    
    }

    @Override
    public String getMailId() {
        if(mailID != null){
            return mailID;
        }else{
            return "Login First";
        }
    }
}

JavaMailingList 

public class JavaMailingList extends MailingListMediator {
    private final String mailingListId = "java";

    @Override
    public String subscribe(MailUser mailUser) {    
        mailingList.add(mailUser);
        return mailingListId;
    }
}

SQLMailingList 

public class SQLMailingList extends MailingListMediator {
    private final String mailingListId = "sql";

    @Override
    public String subscribe(MailUser mailUser) {    
        mailingList.add(mailUser);
        return mailingListId;
    }
}

ForumUsers

public class ForumUsers{
    public static void main(String[] args) {
        MailingListMediator javaMailingList = new JavaMailingList();
        //User 1
        MailUser gmail = new GmailUser();
        gmail.login("[email protected]");
        javaMailingList.subscribe(gmail); // He subscribes

        //User 2
        MailUser yahoo = new YahooUser();
        yahoo.login("[email protected]");
        javaMailingList.subscribe(yahoo); // He subscribes

        //gmail user sends a question to the Java Mailing List
        gmail.sendMailToMailingList(javaMailingList, "How to excel in Java??!!");
    }
}

Does this implementation adhere to the Mediator Design? What improvements can be made?

share|improve this question
    
Post rolled back. Please do not update the original code based on answers as that will invalidate them. More info about that here. –  Jamal♦ Jun 16 '14 at 5:43
    
Oops! Sure. I will make a note of this rule. –  Sowmiya Jun 16 '14 at 5:51

3 Answers 3

up vote 2 down vote

I am not familiar with the Mediator pattern, but I think your code is not complex enough to make good use of the pattern. Take your JavaMailingList and SQLMailingList, what is the difference between those classes ? The value of mailingListId. The implementation of subscribe is exactly the same, you add the same variable to the class. If your MailingListMediator would look like : 

public class MailingListMediator {
    private final String mailingListId;
    protected List<MailUser> mailingList;

    public MailingListMediator(String mailingListId) {
        this.mailingList = new ArrayList<MailUser>();
        this.mailingListId = mailingListId;
    }

    public void sendMail(String message){
        for(MailUser mUser:mailingList){
            System.out.println("Mail message: '"+message+"' sent to "+ mUser.getMailId());
        }
    }
    public String subscribe(MailUser mailUser) {
        mailingList.add(mailUser);
        return mailingListId;
    }
}

There is suddenly no need for JavaMailingList and SQLMailingList.

The same principle apply to YahooUser and GmailUser. Those classes are almost a copy-paste of each other. Same implementation for all the methods and adding the same variable to the class. Why is mailID an attribute in the children classes but not in MailUser? I could repeat the same process that I used for MailingListMediator and I could remove YahooUser and GmailUser.

My conclusion is your code looks like trying to be complicated when in fact it is not. You want to try the Mediator pattern ? Well wait when you will need it, do not force your code into a pattern. Pattern will be useful when you need it, otherwise it will only make your code less readable and harder to maintain.

Since you wanted to implement the Mediator pattern, I will expand more on that part. You do have a mediator pattern implemented. Your "colleagues" do interact between each other through the mediator that is the MailingList. There isn't a MailUser that interact directly with each other, that is the main point of the mediator pattern. Do know that your example is a little weak. I would suggest you, if you really want to test the mediator pattern, try a to code a chat application. This could be a good exercise since you will have a lot of users that will need to communicate, but you don't want the class to be tightly coupled.

share|improve this answer
    
Thanks for that. I get what you are telling. But all I wanted to do is not a real implementation of a Mailing List, but to simulate a scenario to understand how the Mediator Pattern works. –  Sowmiya Jun 13 '14 at 16:41
    
@Sowmiya From what I'm reading of the mediator pattern it looks like a good implementation of the pattern. –  Marc-Andre Jun 13 '14 at 16:59
1  
@Sowmiya this could be a good link to read : stackoverflow.com/questions/373598/… –  Marc-Andre Jun 13 '14 at 17:12
    
Thank you Marc-Andre. –  Sowmiya Jun 15 '14 at 5:45
up vote 2 down vote

login is a setter method and should be named setLogin, or even better, only used in the constructor to make subclasses of MailUser immutable

share|improve this answer
    
Since,its not a real implementation, but just a simulation of Mediator pattern, I have done it with assumption like login logic will differ for different type of mail users. –  Sowmiya Jun 15 '14 at 5:42
    
This you should have a MailUser(address) constructor and a login() instead. –  rds Jun 15 '14 at 10:17
    
Understood.I have done those design changes. –  Sowmiya Jun 16 '14 at 5:42
up vote 0 down vote

In your case, you just have several classes that implement the same MailUser interface. That's just inheritance.

I'm not familiar with mediator pattern, but I see it like a hub. From Wikipedia:

The essence of the Mediator Pattern is to "Define an object that encapsulates how a set of objects interact".

Here, the different MailUsers don't interact with each other.

share|improve this answer
    
Mailusers do interact with each other, through the MailingList. (MailUser can send mails to MailingList, which encloses other(subscribed) MailUsers). –  Sowmiya Jun 15 '14 at 5:21
    
That would be a mediator pattern if the YahooMailUser and GmailMailUser had different interfaces. But here, you simply have a MailingList that contains MailUsers of different implementations. See also Head first on design patterns –  rds Jun 15 '14 at 10:24
    
I get it. But as I see in this link's chatroom example, the Colleague's can be of same type also, is what I understand. Correct me if I am wrong. –  Sowmiya Jun 16 '14 at 5:00

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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