views:

181

answers:

4

Hi, I'm wondering about the standard practice with inner classes (in Java but I suppose it applies to all OO languages). So I have a JFrame subclass ControllerWindow that contains a JPanel subclass MapPanel which I draw onto (so it needs to overwrite paintComponent method) and which needs to implement a mouse listener. My current solution which works is to have MapPanel in a seperate class implementing MouseListener but when I showed this to the guy who runs my course the other day he seemed to think (we have a bit of a language barrier) this should be in an inner class in ControllerWindow or at least the MouseListener should be an inner class.

So my question is what would be the standard solution here, to put a MouseListener in the inner class, the JPanel in a different inner class or still in its seperate class? The JPanel implementing MouseListener in one inner class? And why?

The most important thing to me is that it works but I'd like to know about and understand the standard practices behind these things if possible.

EDIT: Very simplified version of current code below.

class ControllerWindow extends JFrame{
    ...
    MapPanel drawPanel = new MapPanel();
    ...
}

and a separate class:

class MapPanel extends JPanel implements MouseListener{

    ...

    public void paintComponent(Graphics g){
        ...//fillRects etc.
    }

    //MouseListener methods
    public void mouseReleased(MouseEvent e){
        requestFocus();
        ...
        repaint()
        ...
    }
    public void mousePressed(MouseEvent e){}
    public void mouseEntered(MouseEvent e){}
    public void mouseExited(MouseEvent e){}
    public void mouseClicked(MouseEvent e){}
}

Also could this be a situation where it would be acceptable to put both classes in the same file? I don't envisage using MapPanel for anything other than ControllerWindow.

+5  A: 

It is common to use anonymous inner classes as event listeners because the code is usually quite simple (so a separate class may be overkill) and keeping the handler code "close" to the code that registers the listener can improve readability for people trying to understand your code, since all code related to the event is in one place.

EDIT: This is particularly true for classes that implement only one listener method. Perhaps less true for multi-method interfaces like MouseListener, since a class that implements the full interface will be more verbose.

Rob H
For interfaces like `MouseListener` there are classes like `MouseAdapter` that let you override only the methods you really care about.
Bombe
Yup, and this can also be done as an anonymous inner class. Everybody wins.
Rob H
A: 

Because of multiple event handling requirement anonymous inner classes are required. Anonymous class can be written anywhere like in a class, in a method, in the argument. Therefore to abstain from creating many classes for each listener anonymous is preferred.

DKSRathore
+1  A: 

I think it's somewhat arbitrary how you go about it (as Tom Hawtin commented, GUI standards=mud), since you're trading off complexity in the number of classes versus complexity in a single class. If you want to produce code simply for a demonstration, a single file might be easiest. If you want code that you're going to put into production and modify/maintain over time, abstracting out into different classes is almost certainly the way you want to go.

For example, if you embed MapPanel as an inner class in ControllerWindow, and then later want to replace it with a different type of MapPanel, you've got a massive update to ControllerWindow rather than just swapping out MapPanel for a different component type.

With the MouseListener, I'd be inclined to include it in MapPanel if it's handling events specifically for that component (that is, if only the MapPanel "knows" what a click means, it should be the one to process that click). I definitely wouldn't put it in ControllerWindow, since then you're "leaking" implementation detail from MapPanel. (The only case I can think of: in addition to your MapPanel, you have multiple panels type that all need to respond to clicks in the same way, so rather than implementing in each panel you could have the ControllerWindow do it. But even then, I'm not sure the code should be in ControllerWindow).

Whether MapPanel's mouse listener is an inner class implementation of MouseListener, or whether MapPanel implements it (as in your code above) probably comes down to a question of which style you prefer.

Ash
A: 

inner class would be better if it has a simpler syntax.

button1.click( function(event){ do something x...  } );
button2.click( function(event){ do something y...  } );
radio2.check ( function(event){ do something z... } );

java 7 may give us something like that and change the whole situation. as it is now, using a lot of annonymous inner classes can mess up the code and make it impossible to read. you should choose whichever style that makes your code beautiful and legible.

irreputable