Trygve Reenskaug 's MVC specification

Question

I was reading the MODELS - VIEWS - CONTROLLERS paper by Trygve Reenskaug from December 1979 and wanted to try an implementation with Java Swing from my understandings.

Notes I have taken are as follows:

A view is attached to its model (or model part) and gets the data necessary for the presentation from the model by asking questions. It may also update the model by sending appropriate messages. All these questions and messages have to be in the terminology of the model, the view will therefore have to know the semantics of the attributes of the model it represents.

A view should never know about user input, such as mouse operations and keystrokes. It should always be possible to write a method in a controller that sends messages to views which exactly reproduce any sequence of user commands.

A Controller provides means for user input by presenting the user with menus or other means of giving commands and data. The controller receives such user input, translates it into the appropriate messages and pass these messages onto one or more of the views.

The application I worked on is a very simple note-taking application. This is what the application itself looks like:

Directory Layout:

My Model is very simple and knows nothing about Views or Controllers:

package biz.tugay.swingynotes.model;

/* User: [email protected] Date: 2017/05/12 */
public class SwingyNote {

    private final String note;

    public SwingyNote(final String note) {
        this.note = note;
    }

    public String getNote() {
        return note;
    }
}

package biz.tugay.swingynotes.model;

import java.util.*;

/* User: [email protected] Date: 2017/05/12 */
public final class SwingyNoteService {

    private final Set<SwingyNote> swingyNotes = new HashSet<SwingyNote>();

    // Add some sample data..
    {
        final SwingyNote sampleNote = new SwingyNote("Sample Note!");
        addNote(sampleNote);

        final SwingyNote helloWorld = new SwingyNote("Hello World!");
        addNote(helloWorld);
    }

    public void addNote(final SwingyNote swingyNote) {
        swingyNotes.add(swingyNote);
    }

    public Collection<SwingyNote> getAllNotes() {
        return swingyNotes;
    }
}

Moving on to View:

package biz.tugay.swingynotes.view;

import javax.swing.*;
import java.awt.*;

/* User: [email protected] Date: 2017/05/12 */
public class SwingyNotesMainFrame extends JFrame {

    public SwingyNotesMainFrame(final AllNotesPanel allNotesPanel, final AddNewNotePanel addNewNotePanel) {
        super("Swingy Notes");

        // Set Layout..
        final BorderLayout borderLayout = new BorderLayout();
        final Container contentPane = getContentPane();
        contentPane.setLayout(borderLayout);

        // Add panels..
        contentPane.add(allNotesPanel, BorderLayout.CENTER);
        contentPane.add(addNewNotePanel, BorderLayout.SOUTH);

        // Adjust properties and make the frame visible..
        setSize(600, 200);
        setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
        setVisible(true);
    }
}

package biz.tugay.swingynotes.view;

import biz.tugay.swingynotes.model.SwingyNote;
import biz.tugay.swingynotes.model.SwingyNoteService;

import javax.swing.*;
import java.awt.*;
import java.util.Collection;

/* User: [email protected] Date: 2017/05/12 */
public class AllNotesPanel extends JPanel {

    private final SwingyNoteService swingyNoteService;

    public AllNotesPanel(final SwingyNoteService swingyNoteService) {
        this.swingyNoteService = swingyNoteService;
        displayAllNotes();
    }

    public void displayAllNotes() {
        removeAll();
        final Collection<SwingyNote> allNotes = swingyNoteService.getAllNotes();
        for (final SwingyNote swingyNote : allNotes) {
            final JLabel swingyNoteLabel = buildLabelForSwingyNote(swingyNote);
            add(swingyNoteLabel);
        }
        validate();
        repaint();
    }

    private JLabel buildLabelForSwingyNote(final SwingyNote swingyNote) {
        final JLabel swingyNoteLabel = new JLabel(swingyNote.getNote());
        swingyNoteLabel.setBorder(BorderFactory.createLineBorder(Color.BLACK, 2));
        swingyNoteLabel.setFont(new Font("Dialog", Font.BOLD, 27));
        swingyNoteLabel.setOpaque(true);
        swingyNoteLabel.setBackground(Color.YELLOW);
        return swingyNoteLabel;
    }
}

package biz.tugay.swingynotes.view;

import biz.tugay.swingynotes.model.SwingyNote;
import biz.tugay.swingynotes.model.SwingyNoteService;

import javax.swing.*;

/* User: [email protected] Date: 2017/05/12 */
public class AddNewNotePanel extends JPanel {

    private final SwingyNoteService swingyNoteService;
    private final JTextField newNoteInputTextField;

    public AddNewNotePanel(final SwingyNoteService swingyNoteService) {
        this.swingyNoteService = swingyNoteService;
        this.newNoteInputTextField = new JTextField(20);

        add(newNoteInputTextField);
    }

    public void addNote() {
        final SwingyNote swingyNote = new SwingyNote(newNoteInputTextField.getText());
        swingyNoteService.addNote(swingyNote);
        newNoteInputTextField.setText(null);
    }
}

Here is my Controller:

package biz.tugay.swingynotes.controller;

import biz.tugay.swingynotes.view.AddNewNotePanel;
import biz.tugay.swingynotes.view.AllNotesPanel;

import javax.swing.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

/* User: [email protected] Date: 2017/05/12 */
public class AddNewNoteButton extends JButton implements ActionListener {

    private final AddNewNotePanel addNewNotePanel;
    private final AllNotesPanel allNotesPanel;

    public AddNewNoteButton(final AddNewNotePanel addNewNotePanel, final AllNotesPanel allNotesPanel) {
        super("Add New Note!");
        addActionListener(this);

        this.addNewNotePanel = addNewNotePanel;
        this.allNotesPanel = allNotesPanel;
    }

    @Override
    public void actionPerformed(ActionEvent e) {
        addNewNotePanel.addNote();
        allNotesPanel.displayAllNotes();
    }
}

This is how I start my application:

package biz.tugay.swingynotes;

import biz.tugay.swingynotes.controller.AddNewNoteButton;
import biz.tugay.swingynotes.model.SwingyNoteService;
import biz.tugay.swingynotes.view.AllNotesPanel;
import biz.tugay.swingynotes.view.AddNewNotePanel;
import biz.tugay.swingynotes.view.SwingyNotesMainFrame;

import javax.swing.*;

/* User: [email protected] Date: 2017/05/12 */
public class App {

    public static void main(String[] args) {

        // Model..
        final SwingyNoteService swingyNoteService = new SwingyNoteService();

        // View..
        final AllNotesPanel allNotesPanel = new AllNotesPanel(swingyNoteService);
        final AddNewNotePanel addNewNotePanel = new AddNewNotePanel(swingyNoteService);

        // Controller..
        final AddNewNoteButton addNewNoteButton = new AddNewNoteButton(addNewNotePanel, allNotesPanel);
        addNewNotePanel.add(addNewNoteButton);

        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                new SwingyNotesMainFrame(allNotesPanel, addNewNotePanel);
            }
        });
    }
}

Do all components seem to be correct?

Answer 1

I think it would add clarity if AddNewNotePanel was not responsible for the model, being that it is not a view for the model. All this view does is create new SwingyNote objects. You can have the controller be responsible for relaying this change to the AllNotesPanel, making the AllNotesPanel the sole manipulator of the model.

public final class AllNotesPanel extends JPanel {
    ...
    @Override
    public void addNote(final SwingyNote swingyNote) {
        swingyNoteService.addNote(swingyNote);
        displayAllNotes();
    }
    ...
}

In this implementation you have the controller acting as a view. The controller should not be a button, but it may listen to a button. Consider a more basic implementation of the AddNewNotePanel view:

public final class AddNewNotePanel extends JPanel {
    private final JTextField newNoteInputTextField;
    private final JButton addNewNoteButton;

    public AddNewNotePanel() {
        newNoteInputTextField = new JTextField(20);
        addNewNoteButton = new JButton("Add New Note!");
        add(newNoteInputTextField);
        add(addNewNoteButton);
    }

    public String getText() {
        return newNoteInputTextField.getText();
    }

    public void clearText() {
        newNoteInputTextField.setText(null);
    }

    public void addActionListener(final ActionListener listener) {
        addNewNoteButton.addActionListener(listener);
    }
}

This view object exposes a simple set of methods which will be required by the controller to complete its work. The main role of the controller is to simply manipulate the view based on the user input. Here is a possible implementation of this controller:

public final class SwingyController implements ActionListener {
    private final AddNewNotePanel addNewNotePanel;
    private final AllNotesPanel allNotesPanel;

    public SwingyController(
        final AddNewNotePanel addNewNotePanel, 
        final AllNotesPanel allNotesPanel
    ) {
        this.addNewNotePanel = addNewNotePanel;
        this.allNotesPanel= allNotesPanel;
        addNewNotePanel.addActionListener(this);
    }

    @Override
    public void actionPerformed(final ActionEvent e) {
        final SwingyNote swingyNote = new SwingyNote(addNewNotePanel.getText());
        addNewNotePanel.clearText();
        allNotesPanel.addNote(swingyNote);
    }
}

Nitpicks:

SwingyNoteService

I assume the instance initializer in the SwingyNoteService was just to add sample values for testing. I would move that logic to your main method to keep the model code clean.

You should protect your data in the SwingyNoteService#getAllNotes method by creating a defensive copy of the collection, or by wrapping it in an unmodifiable collection. You have no control over what gets added to your model if you return the backing set directly.

public Collection<SwingyNote> getAllNotes() {
    return Collections.unmodifiableSet(swingyNotes);
}

final classes

I like to create my classes as final, and leave them final until I find a case where inheritance is required. eg:

public final class Foo { ... }

Although disabling inheritance prevents you from using one of Java's more powerful features, it's better to have a reason before you allow that type of extension. Designing for inheritance takes a lot more planning and needs to be documented carefully. Inheritance also means that you have to be much more cautious when making changes to this class, because some changes could break subclasses. Further, in most cases I try to favor composition over inheritance

Wildcard imports

Although wildcards are handy, they are generally considered bad practice as it clutters that file's namespace. It could also introduce bugs if in the future these packages add a class which conflicts with your class names, or if two wildcard imports conflict with each-other.