-1

I made a small program using several MVC-patterns. So far I havnt got to much stuff to put in the model so I havnt got any model yet.

My idea is to make one MVC pattern for each panel. And looping through it in the mainloop in the MainController. Then I communicate (starting and stoping) with the MainLoop in the MainController passing the MainController as an parameter to the "subcontrollers" PanOneController and PanTwoController.

Like this:

Main | MainView | MainController (mainloop)

PanelOne Model | PanelOne View | PanelOne Controller

PanelTwo Model | PanelTwo View | PanelTwo Controller

Would this be considered as a good structure? Or some other suggestions of the structure?

This is just an exemple. Im going to add more panels and functions but just want to get the structure right.

Thanks for feedback!

MAIN

import javax.swing.SwingUtilities;

public class Main {

    public static void main(String args[]) {
        SwingUtilities.invokeLater(new Runnable() {

            public void run() {
                MainView theView = new MainView();
                MainModel theModel = new MainModel();
                MainController theController = new MainController(theView, theModel);
                theView.setVisible(true);
            }
        });
    }
}

VIEW

import java.awt.Dimension;
import java.awt.FlowLayout;
import javax.swing.JFrame;
import javax.swing.JPanel;
import panelTwo.PanTwoView;
import panelOne.PanOneView;

public class MainView extends JFrame {

    PanOneView oneV = new PanOneView();
    PanTwoView twoV = new PanTwoView();

    JPanel panelOne = oneV;
    JPanel panelTwo = twoV;

    public MainView() {
        setLayout(new FlowLayout(FlowLayout.LEFT, 100, 10));
        setSize(new Dimension(300, 200));
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        this.add(panelOne);
        this.add(panelTwo);
    }

    public PanOneView getPanelOne() {
        return oneV;
    }

    public PanTwoView getPanelTwo() {
        return twoV;
    }
}

CONTROLLER

import panelOne.*;
import panelTwo.*;

public class MainController implements Runnable {

    private Thread thread;
    private boolean running = false;

    MainView theView;
    MainModel theModel;

    PanOneModel oneM = new PanOneModel();
    PanTwoModel twoM = new PanTwoModel();

    PanOneController oneC;
    PanTwoController twoC;

    private int inc = 0;

    public MainController(MainView x, MainModel y) {
        this.theView = x;
        this.theModel = y;
        thread = new Thread(this);
        oneC = new PanOneController(oneM, x.getPanelOne(), this);
        twoC = new PanTwoController(twoM, x.getPanelTwo(), this);
    }

    public synchronized void start() {
        if (!running) {
            thread = new Thread(this);
            thread.start();
        }
        running = true;
    }

    public synchronized void stop() {
        thread.interrupt();
        running = false;
    }

    // mainloop
    public void run() {
        boolean shifter = false;
        long timer = 0;
        while (running) {
            if (System.nanoTime() - timer >= 1000000) {
                timer = System.nanoTime();
                inc++;
                if (inc % 1000 == 0) {
                    shifter = !shifter;
                }
                oneC.run(shifter);
                twoC.run(shifter);
            }
        }
    }
}

PANELONE VIEW

import java.awt.Color;
import java.awt.Dimension;
import java.awt.event.ActionListener;
import javax.swing.JButton;
import javax.swing.JLabel;
import javax.swing.JPanel;

public class PanOneView extends JPanel {

    private JLabel jl = new JLabel("PANEL 1");
    private JButton start = new JButton("Start");

    public PanOneView() {
        this.setPreferredSize(new Dimension(100, 50));
        this.setBackground(Color.red);
        jl.setForeground(new java.awt.Color(255, 255, 255));
        add(jl);
        add(start);
    }

    public void addTheActionListeners(ActionListener theListener) {
        start.addActionListener(theListener);
    }

    public JButton getStart() {
        return start;
    }

    public void setBg(boolean shifter) {
        if(shifter) {
            this.setBackground(Color.red);
        } else {
            this.setBackground(Color.green);
        }
    }
}

PANELONE CONTROLLER

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

public class PanOneController {
    PanOneView oneV;
    PanOneModel oneM;
    MainController main;

    public PanOneController(PanOneModel m, PanOneView v, MainController main_) {
        this.oneM = m;
        this.oneV = v;
        this.main = main_;
        oneV.addTheActionListeners(theListener);
    }

    public void run(boolean shift) {
        oneV.setBg(shift);
    }

    ActionListener theListener = new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if (e.getSource() == oneV.getStart()) {
                main.start();
            }
        }
    };
}

PANELTWO VIEW

import java.awt.Color;
import java.awt.Dimension;
import java.awt.event.ActionListener;
import javax.swing.JButton;
import javax.swing.JLabel;
import javax.swing.JPanel;

public class PanTwoView extends JPanel {

    JLabel jl = new JLabel("PANEL 2");
    JButton stop = new JButton("Stop");

    public PanTwoView() {
        this.setPreferredSize(new Dimension(100, 50));
        this.setBackground(Color.black);
        jl.setForeground(new java.awt.Color(255, 255, 255));
        add(jl);
        add(stop);
    }

    public void addTheActionListeners(ActionListener theListener) {
        stop.addActionListener(theListener);
    }

    public JPanel getPanelTwo() {
        return this;
    }

    public JButton getStop() {
        return stop;
    }

    public void setBg(boolean shift) {
        if(shift) {
            this.setBackground(Color.black);
        } else {
            this.setBackground(Color.blue);
        }
    }
}

PANELTWO CONTROLLER

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

public class PanTwoController {
    PanTwoView twoV;
    PanTwoModel twoM;
    MainController mcon;

    public PanTwoController(PanTwoModel m, PanTwoView v, MainController mcon_) {
        this.twoV = v;
        this.twoM = m;
        this.mcon = mcon_;
        twoV.addTheActionListeners(theListener);
    }

    public void run(boolean shift) {
        twoV.setBg(shift);
    }

    ActionListener theListener = new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if (e.getSource() == twoV.getStop()) {
                mcon.stop();
                System.out.println("STOP");
            }
        }
    };
}
  • please don't cross-post: stackoverflow.com/questions/40517262/… / codereview.stackexchange.com/questions/146639/… "Cross-posting is frowned upon as it leads to fragmented answers splattered all over the network..." – gnat Nov 10 '16 at 11:58
  • sorry. just not sure exactly where to put my question :-) I have now removed the post from the code review forum. just want to get some feedback if Im heading towards the right direction. Especially regarding passing the MainController as an parameter to the constructor in the PanelOne and PanelTwo controllers – acroscene Nov 10 '16 at 12:03
  • 1
    (I wonder how this question got past the "the question is mostly code" filter... I thought it was already in action). @acroscene: please don't post questions like this one here, they are off-topic. It's unclear, and we don't deal with walls of code here. Please do not post additional code! – Andres F. Nov 17 '16 at 4:14
0

Especially regarding passing the MainController as an parameter to the constructor in the PanelOne and PanelTwo controllers

I would certainly avoid doing this. I would argue that PanOneController and PanTwoController (ie. your sub-controllers) should not be aware of the presence of the MainController. Similarly, PanOneView and PanTwoView should not be aware of the existence of MainView, although that isn't an issue in your code.

By making a subview or subcontroller aware of the "parent" view or controller, you are severely limiting the ability to reuse those components. Instead, I would recommend using listeners to notify up the hierarchy when an action occurs.

Looking at PanOneController and PanOneView, I would start by making a PanOneViewListener interface which will defines methods that are called in response to a user action. That interface can then be implemented by whichever controller is responsible for the business logic behind the action. The view will maintain a collection of these listeners which it will call upon an action being performed.

For example:

public interface PanOneViewListener {
    public void onStartButtonClicked();
}

public class PanTwoView extends JPanel {
    private final List<PanOneViewListener> listeners = new ArrayList<PanOneViewListener>();

    public PanTwoView() {
        ...
        stop.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent arg0) {
                notifyListenersOnStopButtonClicked();
            }
        });
        ...
    }

    private void notifyListenersOnStopButtonClicked() {
        for (final PanOneViewListener listener : listeners) {
            listener.onStartButtonClicked();
        }
    }

    public void addListener(final PanOneViewListener listener) {
        listeners.add(listener);
    }
}

public class PanOneController implements PanOneViewListener {
    public PanOneController(final PanOneModel m, final PanOneView v) {
        ...
        oneV.addListener(this);
        ...
    }
    ...
    @Override
    public void onStartButtonClicked() {
        // Respond to the button being clicked
    }
    ...
}

In your code MainController has the responsibility of handling the thread. So, to remove PanOneController's dependency on MainController, we can again create a listener for PanOneController in much the same way as before:

public interface PanOneControllerListener {
    public void onStartButtonClicked();
}

public class PanOneController implements PanOneViewListener {
    private final List<PanOneControllerListener> listeners = new ArrayList<PanOneControllerListener>();

    public void addListener(final PanOneControllerListener listener) {
        listeners.add(listener);
    }

    @Override
    public void onStartButtonClicked() {
        for (final PanOneControllerListener listener : listeners) {
            listener.onStartButtonClicked();
        }
    }
}

public class MainController implements Runnable, PanOneControllerListener {

    public MainController(final MainView x, final MainModel y) {
        ...
        oneC = new PanOneController(oneM, x.getPanelOne(), this);
        oneC.addListener(this);
        ...
    }

    @Override
    public void onStartButtonClicked() {
        synchronized (this) {
            if (!running) {
                thread = new Thread(this);
                thread.start();
            }
            running = true;
        }
    }
}

This same pattern can be applied to PanTwoView and PanTwoController as well.

Now the views and controllers do not depend on the main view and controller, and can be reused easily. If you were to use PanOneView/PanOneController anywhere else, you would just need the parent to implement the listener of the controller to respond to the actions.

  • thanks! One question. What exactly does the interface do? Im a bit confused over interfaces and abstract classes. – acroscene Nov 16 '16 at 21:34
  • The interface will define a rule/behavior for a class to implement. In this case, we're saying that we want something to happen in response to the start button being clicked. By having an interface, we define the common behavior that multiple classes can implement. Those implementations can be added as listeners and their onStarButtonClicked method will be executed when the start button is clicked. The reason that the controller implements the interface is because in the MVC pattern, the controller is responsible for responding the an action from the view. – avojak Nov 16 '16 at 21:41
  • @acroscene As for the difference between an interface and an abstract class, you should be able to find some good examples by googling a bit :) – avojak Nov 16 '16 at 21:42
  • thanks. yes I know. I have googled already. a bit confused though since abstract and interface class why I need to make this rules and the use of the abstract classes and interfaces but I guess I figure it out sooner or later. Thanks a lot! – acroscene Nov 16 '16 at 21:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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