15

I'm quite new in desktop applications development and I have a pretty big project do deliver this summer. The thing is that the code has to be very clear, so I won't go in (much) trouble when I will update it.

As a result, I want a good "separation of concerns". And the most difficult part to me is the View-Controller separation.

Now, I have read lots of tutorials, discussions etc. And I have designed a mini-app in 3 different ways. The app is simple : click on a button that transform a label to a "Hello world".

What do you think of those 3 designs ?

Is there a better design to meet my expectations ?

Design 1

View1.java :

public View1() {
    initComponents();
    this.controller = new Controller1(this);
}

private Controller1 controller;

public void updateLabel(String message){
    this.jLabel1.setText(message);
}

private void jButton1ActionPerformed(java.awt.event.ActionEvent evt) {
    this.controller.doSomething();
}

private void initComponents() {
...
jButton1.addActionListener(new java.awt.event.ActionListener() {
        public void actionPerformed(java.awt.event.ActionEvent evt) {
            jButton1ActionPerformed(evt);
        }
    });
...}

Controller1.java :

public class Controller1 {
    public Controller1(View1 v){
        this.view = v;
    }

    public void doSomething(){
        this.view.updateLabel("Hello world");
    }

    private View1 view;
}

Design 2

View2.java :

public View2() {
        initComponents();
        this.controller = new Controller2(this);

        jButton1.addActionListener(new java.awt.event.ActionListener() {
            public void actionPerformed(java.awt.event.ActionEvent evt) {
                controller.doSomething();
            }
        });
    }
    public void updateLabel(String message){
        this.jLabel1.setText(message);
    }
    private Controller2 controller;
  ...

}

Controller2.java :

public class Controller2 {

        public Controller2(View2 v){
            this.view = v;
        }

        public void doSomething(){
            this.view.updateLabel("Hello world");
        }

        private View2 view;
}

Design 3

View3.java :

public View3() {
        initComponents();
        this.controller = new Controller3(this);
        this.jButton1.addActionListener(this.controller.listener);
    }
    private Controller3 controller;
    public void updateLabel(String message){
        this.jLabel1.setText(message);
    }
...}

Controller3.java :

public class Controller3 {

    public Controller3(View3 v){
        this.view = v;
        this.listener = new MyListener(v);
    }

    private View3 view;
    public MyListener listener;
}

MyListener.java :

public class MyListener implements ActionListener{
    private View3 view;

    public MyListener(View3 v){
        this.view = v;
    }

    public void actionPerformed(java.awt.event.ActionEvent evt) {
                this.view.updateLabel("Hello world");
            }
}
Koray Tugay
  • 22,894
  • 45
  • 188
  • 319
user777466
  • 991
  • 2
  • 13
  • 21
  • I've been working on the model side of the project. But I'm still wondering about the global design. I'm going to have a lot of data in my view, possibly changed by the user. The best option would be some direct linking between view and model : when a textfield change, then the model object is updated. That would be nice. – user777466 Aug 12 '11 at 09:55
  • You can also look at Swing and how that does it model binding. It is pretty simple and not binded to the view. I do agree that it would be nice for the model to just be updated. But the service that does this updating should probably be a separate class and not in the view. Good luck! – Amir Raminfar Aug 12 '11 at 13:34

4 Answers4

10

I don't like any of these designs. You are coupling the controller to the view to tightly. Let's say you wanted to change controller implementation in the future so you would have to go in all your classes and change the class. Instead you should make it injected. There are a lot of libs that can do this for you via annotations like Guice or Spring but I won't go in to those. Here is a better design.

public class View{
private Controller controller;
   public View(Controller controller) {
       this.controller = controller;
   }
}

This a much cleaner design because the view doesn't have to know what the implementation of the controller is. You can later create a subclass and pass that instead.

So now with the above design I think you can see that you shouldn't pass the View to the controller. This is again coupling which is not good. Instead you can pass an onCallback class that will get executed when it is done. Here is code to undersand it

jButton1.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent evt) {
            controller.doSomething(new Runnable(){
                    public void run(){
                        updateLabel("Hello world");
                    }               
           });
       }
});

Then in your controller do

public void doSomething(Runnable callback){
   // do work
   SwingUtilties.invokeLater(callback);
}

If you look exactly what I have suggested is removing any kind of coupling. The view should not ask for a Controller, it should be given on. The Controller should not know about the view it should just execute a call back. This is important because if you decided to not using Swing, then you wouldn't have all these dependencies on Swing package in your controller.

Hope this all helps!!

Amir Raminfar
  • 33,777
  • 7
  • 93
  • 123
  • See also this [outline](http://stackoverflow.com/questions/2687345/java-mvc-how-to-divide-a-done-text-game-into-mvc/2687871#2687871) and related [example](http://stackoverflow.com/questions/3066590/gui-problem-after-rewriting-to-mvc/3072979#3072979). – trashgod Aug 05 '11 at 21:35
  • Woa, this is cool stuff. If I get it right, you pass a class to the controller and this class is called back by the controller after some other processes. But what if my callback class gets bigger ? Do you recommend to write it within the view.java or in another class ? – user777466 Aug 06 '11 at 10:40
  • If the callback method gets more complicated then the right place to modify is still in the view in my opinion. The idea here is that the view should be responsible of its own callback and not another entity. I do not recommend writing it in another class because the you have to pass all you swing components to other class. Does that accomplish a lot? Try to decouple the class that computes data with the class that update UI. This is a big win in the future when the UI changes. – Amir Raminfar Aug 06 '11 at 18:17
  • @user777466 it's been a while. Did you get a chance to see if this worked for you? – Amir Raminfar Nov 02 '11 at 01:00
6

Deciding which pattern is best depends a lot on the problem you are solving. Swing is already an MVC framework, so you'll have to consider whether adding another layer of indirection on top of it is worth the effort.

Since you are new to UI programming, I suggest you throw together a walking skeleton of your system first, then based on what you learned from that, decide on your architecture. A well-designed architecture makes it easy to test and reuse components. MVP and MVVM are two well-known ways design patterns for UIs.

For your toy problem you could implement either MVP or MVVM as I do below. Keep in mind you also will typically use interfaces between each and will have observers on Model if that can change.

MVP

public class Model {
    public String getWhatIWantToSay() {
        return "Hello World";
    }
}

public class Presenter implements ActionListener {
    private final View view;
    private final Model model;
    public Presenter(Model model, View view) {
        this.model = model;
        this.view = view;
        view.addButtonListener(this);
    }
    public void actionPerformed(ActionEvent e) {
        view.setText(model.getWhatIWantToSay());
    }
}

public class View {
    private JButton button = new JButton();
    private JLabel label = new JLabel();
    public void addButtonListener(ActionListener listener) {
        button.addActionListener(listener);
    }
    public void setText(String text) {
        label.setText(text);
    }
}

MVVP

public class ModelView extends Observable {
    private final Model model;
    private String text = "";

    public ModelView(Model model) {
        this.model = model;
    }

    public void buttonClicked() {
        text = model.getWhatIWantToSay();
        notifyObservers();
    }
}

public class View implements Observer {
    private JButton button = new JButton();
    private JLabel label = new JLabel();
    private final ModelView modelView;

    public View(final ModelView modelView) {
        this.modelView = modelView;
        modelView.addObserver(this);
        button.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                modelView.buttonClicked();
            }
        });
    }

    public void update(Observable o, Object arg) {
        label.setText(modelView.text);
    }
}
James P.
  • 19,313
  • 27
  • 97
  • 155
Garrett Hall
  • 29,524
  • 10
  • 61
  • 76
  • I really like the MVP design you proposed, it's very clear to me. I'm although going to check you link to the Swing Architecture Overview since I sense that there might be some hidden mechanisms that would save me some time. – user777466 Aug 06 '11 at 11:37
  • I'm personally using MVP on a Swing legacy code base at work. Compared to MVVM you have to create more classes and it's a little harder to test, but you get a lot of re-usability. – Garrett Hall Aug 07 '11 at 18:15
  • Good answer but unfortunately links are dead. – Koray Tugay Apr 29 '17 at 19:24
3

I think Design 2 is your best option to meet your criteria.

Problems with Design 1: It is too complex on the view side. The extra methods make it look almost like it has a controller inside of it. Simple changes would become complex to implement.

Problems with Design 3: This pushes too much onto the controller. The controller should not know what Swing events are happening. In that design if you want an action to happen based on a JList instead of a JButton you have to change the view and the controller which is bad.

Other comments about your code:

  • Use import statements so you don't have to include the package of a class in code as in: java.awt.event.ActionListener().
  • You use this. in several places were it is not necessary and that just adds noise.
  • As Amir points out, you have very tight coupling between your view and controller that is not necessary.
jzd
  • 23,473
  • 9
  • 54
  • 76
-1

Another design approach can be something like this:

Model

package biz.tugay.toypro.model;

public interface LabelService {
    String getDateInRandomLocale();
}

package biz.tugay.toypro.model;

import java.text.DateFormat;
import java.util.Calendar;
import java.util.Locale;
import java.util.concurrent.ThreadLocalRandom;

public class LabelServiceImpl implements LabelService {

    private final Locale availableLocalesJava[];

    public LabelServiceImpl() {
        this.availableLocalesJava = DateFormat.getAvailableLocales();
    }

    @Override
    public String getDateInRandomLocale() {
        final int randomIndex = ThreadLocalRandom.current().nextInt(0, availableLocalesJava.length);
        final Locale locale = availableLocalesJava[randomIndex];
        final Calendar calendar = Calendar.getInstance();
        final DateFormat dateFormat = DateFormat.getDateInstance(DateFormat.LONG, locale);
        return dateFormat.format(calendar.getTime());
    }
}

View

package biz.tugay.toypro.view;

import biz.tugay.toypro.model.LabelService;

import javax.swing.*;

public class DateInRandomLocaleLabel extends JLabel {

    private final LabelService labelService;

    public DateInRandomLocaleLabel(final LabelService labelService) {
        this.labelService = labelService;
    }

    public void showDateInRandomLocale() {
        final String dateInRandomLocale = labelService.getDateInRandomLocale();
        setText(dateInRandomLocale);
    }
}

package biz.tugay.toypro.view;

import javax.swing.*;

public class RandomizeDateButton extends JButton {

    public RandomizeDateButton() {
        super("Hit Me!");
    }
}

package biz.tugay.toypro.view;

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

public class DateInRandomLocalePanel extends JPanel {

    public DateInRandomLocalePanel(final JLabel dateInRandomLocaleLabel, final JButton randomizeDateButton) {
        final GridLayout gridLayout = new GridLayout(1, 2);
        setLayout(gridLayout);

        add(dateInRandomLocaleLabel);
        add(randomizeDateButton);
    }
}

package biz.tugay.toypro.view;

import javax.swing.*;

public class MainFrame extends JFrame {

    public void init() {
        setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
        setSize(400, 50);
        setVisible(true);
    }
}

Controller

package biz.tugay.toypro.controller;

import biz.tugay.toypro.view.DateInRandomLocaleLabel;

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

public class RandomizeDateButtonActionListener implements ActionListener {

    private DateInRandomLocaleLabel dateInRandomLocaleLabel;

    @Override
    public void actionPerformed(final ActionEvent e) {
        dateInRandomLocaleLabel.showDateInRandomLocale();
    }

    public void setDateInRandomLocaleLabel(final DateInRandomLocaleLabel dateInRandomLocaleLabel) {
        this.dateInRandomLocaleLabel = dateInRandomLocaleLabel;
    }
}

and finally how I start the Application:

package biz.tugay.toypro;

import biz.tugay.toypro.controller.RandomizeDateButtonActionListener;
import biz.tugay.toypro.model.LabelService;
import biz.tugay.toypro.model.LabelServiceImpl;
import biz.tugay.toypro.view.DateInRandomLocaleLabel;
import biz.tugay.toypro.view.DateInRandomLocalePanel;
import biz.tugay.toypro.view.MainFrame;
import biz.tugay.toypro.view.RandomizeDateButton;

import javax.swing.*;

public class App {

    public static void main(String[] args) {
        final LabelService labelService = new LabelServiceImpl();

        // View
        final DateInRandomLocaleLabel dateInRandomLocaleLabel = new DateInRandomLocaleLabel(labelService);
        final RandomizeDateButton randomizeDateButton = new RandomizeDateButton();

        final DateInRandomLocalePanel dateInRandomLocalePanel = new DateInRandomLocalePanel(dateInRandomLocaleLabel, randomizeDateButton);
        final MainFrame mainFrame = new MainFrame();
        mainFrame.getContentPane().add(dateInRandomLocalePanel);

        // Controller
        final RandomizeDateButtonActionListener randomizeDateButtonActionListener = new RandomizeDateButtonActionListener();

        // Bind Controller to the View..
        randomizeDateButton.addActionListener(randomizeDateButtonActionListener);

        // Bind View to the Controller..
        randomizeDateButtonActionListener.setDateInRandomLocaleLabel(dateInRandomLocaleLabel);

        // Show the main frame..
        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                mainFrame.init();
            }
        });
    }
}

And this is what the application looks like: enter image description here

enter image description here

enter image description here

I think there is no just 1 right answer, it depends on how you would like to tie the components you have..

You might find the following useful as well:

Koray Tugay
  • 22,894
  • 45
  • 188
  • 319