[trunx]

Hi all, please for give me if I don't make to much sense. New to this forum and a novice programmer!

I'm stuck on an assignment question (studying at 31, crazy I know!)

The problem is this (there will be others!)

I have a JMenuBar with 4 JMenus in it. Each JMenu has some JMenuItems. I need to add ActionListeners to each menu item so that I can make them do something when clicked. For now I just want the Exit menu item to close the application, but it does nothing. Here is my code-

Constructor with code to create the menu

public M257DrawingApplication(String title)
    {
        super(title);
        setSize(FRAME_WIDTH, FRAME_HEIGHT);
        this.setResizable(false);
        setLayout(new BorderLayout());
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        JMenuBar mb = new JMenuBar();
        JMenu m1 = new JMenu("Style");
        JMenu m2 = new JMenu("Size");
        JMenu m3 = new JMenu("Colour");
        JMenu m4 = new JMenu("Close");
        
        m1.add(new JMenuItem("Courier New"));
        m1.add(new JMenuItem("Courier New"));
        m1.add(new JMenuItem("Courier New"));
        m2.add(new JMenuItem("16"));
        m2.add(new JMenuItem("18"));
        m2.add(new JMenuItem("20"));
        m3.add(new JMenuItem("Blue"));
        m3.add(new JMenuItem("Red"));
        m3.add(new JMenuItem("Green"));
        m4.add(new JMenuItem("Exit"));
        mb.add(m1);
        mb.add(m2);
        mb.add(m3);
        mb.add(m4);
        setJMenuBar(mb);

Adding an ActionListener to m4(the Close menu)

m4.addActionListener(mw);

My inner ActionListener class

public class MenuWatcher implements ActionListener
        {
           public MenuWatcher()
           {
              super();
           }

           public void actionPerformed(ActionEvent e)
           {
               if(e.getSource() == "Exit")
               {
                  System.exit(1);
               }
           }

         }

I cant figure out what I'm doing wrong.
All advice will be much appreciated.
Thanks..

[Mylo]

One important thing to note here is that getSource() returns the invoking object, which is not a string in this case. You can cast the object type to a JMenuItem and perform any one of JMenuItems methods that would be equal to that string. (I assume you are trying to compare text) I'd recommend not comparing text personally, but the object, since the text may be changed at run time.

And just for future referece, you should use .equals over == when comparing objects.

[trunx]

Would it be better to make a variable called exit to reference the JMenuItem("Exit" ) and use

if (e.getSource.equals(exit))

[trunx]

Well that didn't work. NetBeans tell me that it can not find variable 'exit'............

[smohd]

The problem is that this condition is not correct:

 if(e.getSource() == "Exit")

As said getSource returns the object which the event occurred, so you have check if it is equals to the name of the source you want. For example if exit menu item is called ExitMenu, then:

if(e.getSource() == ExitMenu)

Also you can use getActionCommand() and set command action for every control

[Mylo]

That is because exit is declared in another class, if it's not that, then it's becaused you declared your variables inside the constructor and is hence not able to be seen by other methods.

Spoiler

I'm have little knowledge of separating GUI from code, so beware I may answer this in the wrong manner. --- You could pass your M257DrawingApplication class to the MenuWatcher class via the constructor.

MenuWatcher mw = new MenuWatcher(this) // this being the class you pass in so menuWatcher has a reference to this class

public class MenuWatcher implements ActionListener
        {
           private M257DrawingApplication gui;

           public MenuWatcher(M257DrawingApplication m)
           {
              gui = m;
           }

           // you can then use the gui variable to access your gui.

         }

There is then the problem of you having put your gui in a method. You could either use the .getJMenuBar() method (it might be different or not exist, i'n not sure) which you should be able to access each sub item of the JMenuBar.

Otherwise you can declare you GUI items as instance fields.

There is definately better ways to do it than that, but that is all I know of. I suggest you read StateManager and MVC to improve your design.

Edit: Yikes, completely forgot about setActionCommand, that would definately be easiest. Ignore me

This post has been edited by Mylo: 05 February 2012 - 08:39 AM

[smohd]

Oracle has a tutorial here on menu. But what I see in your code:
-You declare your menu and menu items inside a constructor, said this.
- You add action listener to only Jmenu, you have to add to menu items also.
- Your menu items has no reference name because you add them directly. I would prefer giving them them and declare them as class variables so you can use names in your inner class. Look at the tutorial you will see how they do that.
Then just as buttons, you can check for the getSource() to see which menu item is clicked.

[trunx]

Thanks for your help guys, I appreciate it.

So where should I create the menu and menu items if not in the constructor?
Thanks.

[smohd]

Declare them as class variables then you can instantiate inside your constructor. SO you will have something like:

public class M257DrawingApplication....{
   //declare reference here
   JMenu m1;
   //and so
   public M257DrawingApplication(){
       //then inside constructor
     m1 = new JMenu("Style");
//and so

[trunx]

Here is what I have done now. It is still not doing anything when I click on Exit in the Close menu.

public class M257DrawingApplication extends JFrame
      {
   

    private final int FRAME_WIDTH = 1000;
    private final int FRAME_HEIGHT = 500;
    private DrawingPanel drawingPanel;
    private MenuWatcher mw;
    JMenu m1;             //Is this what you meant?
    JMenuItem exit;

Then in the constructor

public M257DrawingApplication(String title)
    {
        super(title);

        setSize(FRAME_WIDTH, FRAME_HEIGHT);
        this.setResizable(false);
        setLayout(new BorderLayout());
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        JMenuBar mb = new JMenuBar();
        m1 = new JMenu("Style");  //instantiate  the menu m1
        JMenu m2 = new JMenu("Size");
        JMenu m3 = new JMenu("Colour");
        JMenu m4 = new JMenu("Close");
        
        m1.add(new JMenuItem("Courier New"));
        m1.add(new JMenuItem("Courier New"));
        m1.add(new JMenuItem("Courier New"));
        m2.add(new JMenuItem("16"));
        m2.add(new JMenuItem("18"));
        m2.add(new JMenuItem("20"));
        m3.add(new JMenuItem("Blue"));
        m3.add(new JMenuItem("Red"));
        m3.add(new JMenuItem("Green"));
        exit = new JMenuItem("Exit"); // instantiate  the menu item exit

        m4.add(exit);
        mb.add(m1);
        mb.add(m2);
        mb.add(m3);
        mb.add(m4);
        setJMenuBar(mb);
       

        //set visible here, now size is available
        setVisible(true);
        drawingPanel = new DrawingPanel(getAvailableWidth(), getAvailableHeight());
        
        

        // given exiting on close
        setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
        //TODO add registering of any event handlers here
        exit.addActionListener(mw);
       
        
       
        

    }

The inner MenuWatcher code

public class MenuWatcher implements ActionListener
        {
           

           public void actionPerformed(ActionEvent e)
           {
               Object event = e.getSource();
               if(event.equals(exit))
               {
                  System.exit(1);  //Is any of this correct?
               }
           }

         }

Thanks..

[trunx]

Solved. Thanks for the help guys.............