1

I'm trying to make a GUI to generate characters for a tabletop RPG, using MVC coding practices. I will have several "control panel" classes, each with a particular set of buttons to handle the different aspects of the character creation process (rolling attributes, buying equipment, etc). My Controller class will have as members an instance of each "control panel" class. My generalized code is here:

public class Controller{

private Model m; //class Model not shown
private Viewer v; //class Viewer not shown
private JFrame frame;
private ControlPanelOne cpo;
private ControlPanelTwo cpt; //class ControlPanelTwo not shown

public Controller(){
    m = new Model();
    v = new Viewer();
    frame = new JFrame();
    frame.setLayout(…);
    cpo = new ControlPanelOne();
    cpt = new ControlPanelTwo();
    frame.add(cpo.getPanel());
    frame.add(cpt.getPanel());}

public void update(){
    m.update();
    v.update();}}

public class ControlPanelOne{

private JPanel panel;
private JButton button;

public ControlPanelOne(){
    panel = new JPanel();
    button = new JButton(“press me”);
    panel.add(button);
    button.addActionListener(new ActionListener(){
        public void actionPerformed(ActionEvent e){
            //do some stuff;
            //call Controller’s update method;}})} //the important bit!

public JPanel getPanel(){
    return panel;}}

public class Tester{

public static void main(String[] args){

    Controller c = new Controller();}}

How can I get ControlPanelOne to see and call on Controller’s update() method when ControlPanelOne’s button is pressed?

I tried making ControlPanelOne extend Controller so I could call super.update() when ControlPanelOne’s button is pressed, but then when a Controller is instantiated (such as in the Tester class), its ControlPanelOne member is also instantiated, whereby both Controller and ControlPanelOne’s constructors are called in an infinite loop of construction.

Trying to trick the compiler by making Controller’s update() method static doesn’t work because instances m and v aren’t static.

I don’t want to make ControlPanelOne an inner class of Controller if I can help it. There will be other “control panel” classes (ControlPanelTwo, etc.), and I’m afraid making them all inner classes would make Controller’s code long and messy. I would like each “control panel” as a separate class to handle one particular function for neatness’s sake, and for ease of maintenance and modular code reuse.

It seems clear that ControlPanelOne shouldn’t have a Controller as a member variable, because a) it’s conceptually backwards and b) instances of the other “control panels” would need their own instances of a Controller…

I used to have Controller extend JFrame and ControlPanelOne extend JPanel (so that Controller could add() an instance of ContollPanelOne directly instead for calling ControlPanelOne’s getPanel() method), but I read making classes extend such components instead of making such components members of your class is poor form. But if it would somehow help to go back to having Controller and ControlPanelOne extend JFrame and JPanel…

Perhaps my idea of MVC is wrong...

Please advise

zizkovo
  • 13
  • 3
  • You should use interfaces to define common methods like `update` and pass that interface as a parameter to the control panel – OneCricketeer Apr 23 '16 at 00:18
  • If you want a specific instance of Class A to be able to call instance methods on a specific instance of Class B, then why not simply pass a reference to the B instance to the constructor for A? – scottb Apr 23 '16 at 00:31

4 Answers4

1

Indeed, you're not adhering to the MVC pattern. The gist of that pattern is to provide clean separation of your models, views, and controllers. So, any attempt to inherit or compose them of each other is defeating the benefits.

What I would do is make an AbstractAction like:

public class UpdateAction extends AbstractAction { 

    private Controller controller;

    ...
    @Override
    public void actionPerformed(ActionEvent ae) {
        controller.update();
    }
}

Then, initialize and set the button up outside the view, the view need not know what it's displaying; it only needs to know that its job is to display things. You could have the controller extend JFrame and add the JPanels to it.

JButton button = new JButton("Button");
button.setAction(new UpdateAction(this));
... more button set up
... add button to views
... add views to frame

Now, whenever the button is clicked it will call the controller's update() method.

ChiefTwoPencils
  • 13,548
  • 8
  • 49
  • 75
1

How can I get ControlPanelOne to see and call on Controller’s update() method when ControlPanelOne’s button is pressed?

This is generally achieved through the use of an Observer Pattern, where the controller will register interest with the view to be notified when certain events occur, the view will, when appropriate, call each registered party when the event occurs. This way, the view doesn't care about the controller, only that it needs to generate appropriate events.

MadProgrammer
  • 343,457
  • 22
  • 230
  • 366
  • It's true what you say here about observer, but isn't it putting too much code in the view, being that it now cares about specific events? If you had various key bindings, button and mouse events, etc., putting that into the view so it can notify the controller is less flexible. The view, as I write them, simply draw the objects given to it that it's responsible for (simply go.draw(g)). It listens to the controller for changes (e.g. player movement) and `repaint()`s.Benefit being you can change behavior based on game state which the view should also not know about. – ChiefTwoPencils Apr 23 '16 at 01:24
  • I've seen people try and force Swing into a pure MVC, but Swing is already a form of MVC. Instead, I allow the view to be "what ever" it needs to be and the define a contract between the view and the controller about the expected functionality they are expected to provide, this way, I can change the view in whatever way I want and so long as it confirms to the expected contract of the view. Why does the view need to know about the controller? In fact, the way I write MVC in Swing, the view and model know nothing about the controller at all, they just generate events the controller responds to – MadProgrammer Apr 23 '16 at 01:45
  • It's the responsibility of the view to represent the state of the model (via information provided to it by the controller), so the view is now free to do that in whatever manner it wants and the controller doesn't care, so if I have to change the view for some reason, I don't need to write a brand new controller to support it. With this approach I've completely decoupled each layer, as it's only bound by the contracts they define, I can replace the model and/or the controller and/or the view individually and the whole thing keeps on working – MadProgrammer Apr 23 '16 at 01:48
  • My other issue is, I don't like exposing UI components to outsiders, I've seen too many people make the UI controls public or accessible directly via getters, thereby throwing away the concept of encapsulation and responsibility. What does the controller care if I just a series of `JRadioButton`s or `JToggleButton`s or a `JSlider`, all it should care about is some state of selection changed – MadProgrammer Apr 23 '16 at 02:05
  • I don't think there is a "right" way to wrap a MVC around Swing, you need to approach each situation based on your context and needs – MadProgrammer Apr 23 '16 at 02:41
0

If I understood well class B is a component of class A
The class B does not know about class A, even if it is member of that class, it has no refference to it. This is called Composition is a "has-a" relationship. If you want to know about that class you should also have an instance of class A in class B, or make the method of class A static.

class B {
    btnPressed() {
        /*To call this method you need or an object of class A, or static method in class A*/
        methodA(); //Method form class A

        op1:
        a = new A();
        a.methodA();

        op2:
        //make methodA static in class A 
        A.methodA();
    }
}

class A {
    b = new B();

    methodA();
}

Hope this helps.

V. Sambor
  • 12,361
  • 6
  • 46
  • 65
0

Your immediate objective is to invoke update on an instance of Controller within the actionPerformed method of an anonymously implemented ActionListener within ControlPanelOne's constructor. ControlPanelOne is composed within the constructor of an instance of Controller. Effectively, you need a circular reference from Controller to ControlPanelOne and visa versa.

Here's a straight forward approach. You can check out the minimally worked out project here.

First, your Controller. 

public class Controller {

    private Model m; //class Model not shown
    private Viewer v; //class Viewer not shown
    private JFrame frame;
    private ControlPanelOne cpo;
    private ControlPanelTwo cpt; //class ControlPanelTwo not shown

    public Controller() {
        m = new Model();
        v = new Viewer();
        frame = new JFrame();
        frame.setLayeredPane(null); // Do something

        cpo = new ControlPanelOne(this); // Supply self to the instance of ControlPanelOne
        cpt = new ControlPanelTwo();

        frame.add(cpo.getPanel());
        frame.add(cpt.getPanel());
    }

    public void update() {
        m.update();
        v.update();
    }
}

Next, ControlPanelOne:

public class ControlPanelOne {

    private JPanel panel;
    private JButton button;

    public ControlPanelOne(final Controller controller){
        panel = new JPanel();
        button = new JButton("press me");
        panel.add(button);
        button.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                //do some stuff;

                controller.update();

            }
        });
    }


    public Component getPanel() {
        return null;
    }
}

The important thing to remember is to declare the supplied instance of Controller final in the constructor (you can find out why here). This isn't necessary in Java 8, but it's still a good habit to get into.

Community
  • 1
  • 1
revprez
  • 1,016
  • 7
  • 8