9 replies to this topic

[AfroJack]

Hello, I'm making a GUI for college, and it's only my first week back, and my first week doing swing, so please excuse the bad programming and stuff, the code is not expected to be perfect. I haven't yet figured out how to clean up code for action listener, and this is what I have so far.. is this what it should look like ? or is this bad code.. Basically the GUI has 10 buttons, and this is the action handler for the 10 buttons, I realise this looks terrible, but that's what brought me here :D anyways, thanks for any help which I can get.

else if (e.getActionCommand().equals("France #1"))
{
Wfield.setText("France");
}
else if (e.getActionCommand().equals("France #2"))
{
RUfield.setText("France");
}
else if (e.getActionCommand().equals("Brazil #1"))
{
Wfield.setText("Brazil");
}
else if (e.getActionCommand().equals("Brazil #2"))
{
RUfield.setText("Brazil");
}
else if (e.getActionCommand().equals("Spain #1"))
{
Wfield.setText("Spain");
}
else if (e.getActionCommand().equals("Spain #2"))
{
RUfield.setText("Spain");
}
else if (e.getActionCommand().equals("Portugal #1"))
{
Wfield.setText("Portugal");
}
else if (e.getActionCommand().equals("Portugal #2"))
{
RUfield.setText("Portugal");
}
else if (e.getActionCommand().equals("England #1"))
{
Wfield.setText("England");
}
else if (e.getActionCommand().equals("England #2"))
{
RUfield.setText("England");
}
}

---------- Post added at 11:00 PM ---------- Previous post was at 10:52 PM ----------

If it doesn't need to be cleaned up and this is correct, please post saying so :) thanks

[gregwarner]

If your action handlers are simple and small enough, it should suffice to add quick little anonymous action handlers to each button as you go. That way it keeps things clean and separated for you.

JButton button1 = new JButton("Button #1");

button1.addActionListener(new ActionListener() {

    @Override

    public void actionPerformed(ActionEvent e) {

        // Do action for Button #1.

    }

});


JButton button2 = new JButton("Button #2");

button2.addActionListener(new ActionListener() {

    @Override

    public void actionPerformed(ActionEvent e) {

        // Do action for Button #2.

    }

});


JButton button3 = new JButton("Button #3");

button3.addActionListener(new ActionListener() {

    @Override

    public void actionPerformed(ActionEvent e) {

        // Do action for Button #3.

    }

});


// ... and so on and so forth.

It's up to you whether you do it the above way, or if you create one action listener to rule them all, and test which button called it. I've seen both methods done.

EDIT: Of course, if all you're trying to do is retrieve a certain string based on which button was pressed, you could do this with one action listener in constant time by using a HashMap, using the button as the key and the target string as the value. Would be much simpler as it would only require one action listener and zero if statements.

[lethalwire]

Can you use JRadioButtons?

[Norm]

You could save a clientPropery value that is an index into an array of names with each button and use that. Something like this:
final String IndexName = "TheIndex";

...
button.setClientProperty(IndexName, 1); // set the index for this country name
...
int ix = xxx.getClientProperty(IndexName); // xxx is the component that sent the event. get it from getSource()
tf.setText(countryNames[ix]);

[lethalwire]

Norm said:

You could save a clientPropery value that is an index into an array of names with each button and use that. Something like this:
final String IndexName = "TheIndex";

...
button.setClientProperty(IndexName, 1); // set the index for this country name
...
int ix = xxx.getClientProperty(IndexName); // xxx is the component that sent the event. get it from getSource()
tf.setText(countryNames[ix]);

How would you distinguishing winner countries from runner up countries?

[Norm]

Use another property.
OR have two actionListeners: One for winner and one for runner up

[lethalwire]

Norm said:

Use another property.
OR have two actionListeners: One for winner and one for runner up

I see.

[AfroJack]

Thanks everyone for the reply, but I've just went with what gregwarner said, and then I can hide the full method so it's cleaned up to just
FWbutton.addActionListener(new ActionListener() { ...
FRUbutton.addActionListener(new ActionListener() { ...
5 times for each button! Thanks for all the help

[lethalwire]

AfroJack said:

Thanks everyone for the reply, but I've just went with what gregwarner said, and then I can hide the full method so it's cleaned up to just
FWbutton.addActionListener(new ActionListener() { ...
FRUbutton.addActionListener(new ActionListener() { ...
5 times for each button! Thanks for all the help

I can see this being done more easily using JRadioButtons, 1 array of country names, and 1 loop.

final String[] COUNTRIES = {"France", "Brazil", "Portugal", "England"};

private final JTextField WINNER_FIELD = new JTextField(11), RUNNER_UP_FIELD = new JTextField(11);



setup() {

    setupButtonPanel(WINNER_FIELD) // for winner

    setupButtonPanel(RUNNER_UP_FIELD) // for runner up

}


...


setupButtonPanel(JTextField textField) {

   ButtonGroup group...

   JPanel panel...

   for(each country in COUNTRIES) {

      JRadioButton btn = new JRadioButton(country);

      btn.addActionListener( ...

         actionPerformed(...

             textField.setText(btn.getText())

       group.add(btn);

       panel.add(btn)

    }

   this.add(panel);

}

Then the check to see if the winner field equals the runner up field is as easy as:

if( WINNER_FIELD.getText().equals(RUNNER_UP_FIELD.getText())

[AfroJack]

That worked! Thanks :D