views:

77

answers:

2

I have code similar to the following:

public class myButton extends JButton()
{
    public int data;
    public myButton(){
        super("asdf");
        data = 2;
    }
}

public class myPanel extends MouseListener()
{
    myButton myButtonVar1;
    myButton myButtonVar2;
    public myPanel()
    {
        myButtonVar1 = new myButton();
        myPanel.add(myButtonVar1);
        myButtonVar1.addMouseListener(this);
        myButtonVar2 = new myButton();
        myPanel.add(myButtonVar2);
        myButtonVar2.addMouseListener(this);
    }

    //MouseListener Methods are here
    void mouseClicked(MouseEvent e)
    {
        //say this changes the myButton that was clicked data based off
        //the other myButton's data
        doSomething((myButton)(e.getSource()).data);
    }
}

And then I add that panel to a JFrame via setContentPane.

This works alright. The handler has to have access to all of the buttons, because it needs to know myButtonVar1.data and myButtonVar2.data

This setup doesn't sit right with me, but the main problem is that I have to have other buttons in the frame as well, which access the myButtons.

So how could I clean this up such that I could add something a "resetButton" that would reset all of the myButtons contained in myPanel. The route that stands out to me would be to use instanceof to see if the source is a resetButton or myButton, but that seems to be strongly discouraged in everything i've seen.

I hope I'm not too far off here. My goal is to write good code rather than stick with what I have, so let me know if I have done something fundamentally wrong already and should backtrack.

A: 

Classes should start with an uppercase letter (MyButton instead of myButton). This is a convention.

Fields are usually private, and you only have a getter (myButton.data).

instanceof is seldom required. Instead, you could define a base class with a "press" method. Both classes could then implement it.

Thomas Mueller
Yeah, I wrote that quickly, I normally follow the capitalization conventions and use accessors and mutators but I thought I'd try to keep it concise. I probably should have been more careful when asking for general advice.As far as the "press" method goes, that is what seemed like the way to do it, the problem is that the action being performed requires knowledge of the other buttons, and therefore can't be performed as a method of the button. I could pass in the panel in the constructor I suppose, giving me access to it in it's entirety. myButton(this);
Kyle
+2  A: 

In general, you can fix your problem by preferring composition over inheritance. Your buttons should not be special class buttons. Instead they should be raw JButtons. You can create a class that returns a JButton with the preferred settings and also holds the data, but try not to extend the Swing classes.

For your encapsulation issue, don't make data public. Instead, have a getData() getter to get the value as needed.

You won't need to use instanceof. Instead, install two different MouseListeners - one for your MyButton buttons and one for your ResetButton buttons. This can be done with external classes or a private inner class that implements MouseListener. All MyButton-type buttons will have the one type of listener, while the single ResetButton will have the other.

justkt