Search code examples
javaswingevent-handlingactionlistenerjmenubar

JAVA: Swing - actionPerformed() on JMenuItem fires but updated values do not Persist


Ok, so I have been building a simple Java Swing based GUI for an opengl graphics engine I've been developing.

The problem I'm having is that the menu bar is not functioning as I would like. Specifically, I have a MenuBarBuilder class that builds the various menus, sub-menus and menu-items. When a menu-item is added, it is assigned a new actionEventListener and given a simple task of setting a Boolean value to true; this Boolean value is stored in a list of states associated with that menu. Each menu Item state is created when adding a menu Item to a menu. The states are stored in the 'StatesList' class as an object of type 'State'.

In the MenuBarBuilder class each of the menu-items that need to function as a unit are added as a 'group'. When this is done a second action event listener is added to each menu-item that sets all the associated values of the menu group to false. Ultimately this has the effect of making sure only one menu-item in the group has a state in the states list that is set to true.

When a menu item is clicked the action listener events are fired. When fired they print out a list to determine the order they are fired in and to make sure the state values are set as expected, see below:

setfalse
list
MenuItem1Name: false
MenuItem2Name: false

MenuItem1Name
setTrue
list
MenuItem1Name: true
MenuItem2Name: false

setfalse
list
MenuItem1Name: false
MenuItem2Name: false

MenuItem2Name
setTrue
list
MenuItem1Name: false
MenuItem2Name: true

This ensures that the events are firing and in the correct order. We can see that when the MenuItem1Name Button is Pressed, the values are correctly set to false and the relevant field is subsequently updated to true. The same can be seen for MenuItem2Name. Now to get to the actual Problem.

When I get the state list and check the values in a loop via the GUI class, I only ever get 'false' values from the state list. I have implemented this pattern in a similar fashion using Swing toolbars which works fine but I simply cannot fathom why I am unable to retrieve the values that are set correctly by the action listener events. I can see no place where the values are overwritten or where there is a strange order/mixing of object instantiation.

So, the Question is: why do I only ever get false when testing the state list?

UPDATED: attempt at SSCCE


Here is UML class Diagram with the relevant classes

Below is the implementation of the relevant classes.

State.java

public class State {

    private String name;
    private Object value;

    State(String name, Object value){
        this.name=name;
        this.value=value;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    public Object getValue() {
        return value;
    }

    public void setValue(Object value) {
        this.value = value;
    }

    public String toString(){   
        return getName() + ": " + getValue();   
    }

}

StatesList.java

import java.util.ArrayList;

public class StatesList {

    private ArrayList<State> states;

    StatesList(){
        states= new ArrayList<State>();
    }

    void addState(String name, Object value){
        states.add(new State(name, value));
    }

    State getState(String name){
        for(State s : states){
            if(s.getName().equals(name)){
                return s;
            }
        }
        return null;
    }

    ArrayList<State> getStates() {
        return states;
    }

    public String toString(){
        String rString="";
        rString+="list\n";
        for (State s : states) {
            rString+=s.toString()+"\n";
        }
        return rString;
    }

}

MenuBar.java

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JMenu;
import javax.swing.JMenuBar;
import javax.swing.JMenuItem;

public class MenuBar {

    private JMenuBar menuBar;
    private StatesList menustates;

    public MenuBar() {
        menuBar = new JMenuBar();
        menustates = new StatesList();
    }

    void createMenu(String title) {
        JMenu menu = new JMenu(title);
        menuBar.add(menu);
    }

    void createMenuItem(JMenu menu, String itemTitle) {

        JMenuItem menuItem = new JMenuItem(itemTitle);

        menustates.addState(itemTitle, false);

        menuItem.addActionListener(new ActionListener() {
            int menuItemindex = menustates.getStates().size()-1 ;

            @Override
            public void actionPerformed(ActionEvent actionEvent) {
                System.out.println(actionEvent.getActionCommand());
                menustates.getStates().get(menuItemindex).setValue(true);
                //print list of current states
                System.out.println("setTrue");
                System.out.println(menustates.toString());
            }
        });
        menu.add(menuItem);
    }

    //not a real group, just ensures MenuItem behaves the same as other menu items
    void addMenuGroup(JMenuItem[] jMenuItems, int startComponentIndex) {    

        for (int i = 0; i < jMenuItems.length; i++) {
            jMenuItems[i].addActionListener(new ActionListener() {
                @Override
                public void actionPerformed(ActionEvent e) {
                    for (int i2 = 0; i2 < jMenuItems.length; i2++) {
                        //make sure each state related to the menuItemGroup is to false
                        menustates.getStates().get(i2 + startComponentIndex).setValue(false);
                    }
                    //print list of current states
                    System.out.println("setfalse");
                    System.out.println(menustates.toString());
                }
            });
        }
    }

    JMenuBar getMenuBar() {
        return menuBar;
    }

    public StatesList getStates() {
        return menustates;
    }


}

MenuBarBuilder.java

import javax.swing.JMenuBar;
import javax.swing.JMenuItem;

public class MenuBarBuilder {

    private MenuBar DataViewMenuBar;

    MenuBarBuilder() {
        DataViewMenuBar = buildDataViewMenu();
    }

    MenuBar buildDataViewMenu() {
        MenuBar tb = new MenuBar();

        //create menu
        tb.createMenu("TestScenes");

        //add two menuItems
        tb.createMenuItem(tb.getMenuBar().getMenu(0), "MenuItem1Name");
        tb.createMenuItem(tb.getMenuBar().getMenu(0), "MenuItem2Name");

        //add buttons two a 'group' sort of
        tb.addMenuGroup(new JMenuItem[] { 
                (JMenuItem) tb.getMenuBar().getMenu(0).getItem(0),
                (JMenuItem) tb.getMenuBar().getMenu(0).getItem(1), },
                tb.getStates().getStates().size() - 2);

        return tb;
    }

    StatesList getDataViewMenuBarStates() {
        return DataViewMenuBar.getStates();
    }

    JMenuBar getDataViewMenuBarMenubar() {
        return DataViewMenuBar.getMenuBar();
    }

}

GUI.java

import javax.swing.JFrame;

public class GUI {

    MenuBarBuilder mbb;
    boolean Selected = false;

    GUI() {
        mbb = new MenuBarBuilder();
    }

    void run() {
        final JFrame frame = new JFrame("Main");
        frame.setJMenuBar(mbb.buildDataViewMenu().getMenuBar());

        frame.pack();
        frame.setVisible(true);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        while (!Selected) {
            selectLoadScene();
        }
        System.out.print("bap- selection must have occured");
    }

    void selectLoadScene() {
        String selectedTest = "";


    // The Test Occurs here
    // loop through states list
        System.out.println(mbb.getDataViewMenuBarStates().getStates().toString());
        for (State s : mbb.getDataViewMenuBarStates().getStates()) {
            // check the value for each state -this value is always false,..
            // thats the problem
            if ((boolean) s.getValue() == true) {
                // if true save value
                selectedTest = s.getName();
                break;
            }
        }

        // test selected value and see if it matches the following cases
        switch (selectedTest) {
        case "MenuItem1Name":
            // dosomething
            Selected = true; // breaks while loop
            break;
        case "MenuItem2Name":
            // dosomething
            Selected = true;// breaks while loop
            break;
        default:
            // do nothing
        }
    }
}

Start.java

public class Start {

    GUI gui;

    public static void main(String[] args) {
        GUI gui= new GUI(); 
        gui.run();
    }

}

Solution

  • The menubar in while loop is different from the menubar in the action performed methods. why? If you carefully look at the following 2 lines from class GUI, you will come to know.

    mbb = new MenuBarBuilder();
    frame.setJMenuBar(mbb.buildDataViewMenu().getMenuBar());
    

    Here 2 different menubars are created. as constructor of MenuBarBuilder internally calls buildDataViewMenu(), so 2 calls to "buildDataViewMenu()". which will end up creating 2 menubars.

    To avoid this

    Dont call "buildDataViewMenu()" in

    frame.setJMenuBar(mbb.buildDataViewMenu().getMenuBar());
    

    As you just want the menubar, you should manage the code in such a way that you will get the handle for the this.tb = new MenuBar() may be you can add the getter and setter for the same.

    When I added the getter and setter for the same as bellow it started working.

    public class MenuBarBuilder {
    
    private MenuBar DataViewMenuBar;
    MenuBar tb;
    
    MenuBarBuilder() {
        DataViewMenuBar = buildDataViewMenu();
    }
    
    MenuBar buildDataViewMenu() {
       this.tb = new MenuBar();
    
        //create menu
        tb.createMenu("TestScenes");
    
        //add two menuItems
        tb.createMenuItem(tb.getMenuBar().getMenu(0), "MenuItem1Name");
        tb.createMenuItem(tb.getMenuBar().getMenu(0), "MenuItem2Name");
    
        //add buttons two a 'group' sort of
        tb.addMenuGroup(new JMenuItem[] { 
                (JMenuItem) tb.getMenuBar().getMenu(0).getItem(0),
                (JMenuItem) tb.getMenuBar().getMenu(0).getItem(1), },
                tb.getStates().getStates().size() - 2);
    
        return tb;
    }
    
    public MenuBar getMenuBar(){
        return this.tb;
    }
    
    StatesList getDataViewMenuBarStates() {
        return DataViewMenuBar.getStates();
    }
    
    JMenuBar getDataViewMenuBarMenubar() {
        return DataViewMenuBar.getMenuBar();
    }
    
    }
    

    and in class GUI call should be like,

    frame.setJMenuBar(mbb.getMenuBar().getMenuBar());
    

    This will resolve the issue. Let me know if you are facing any issue.