# Action Event and checking buttons

I am creating a simple program with multiple buttons. Is this way of checking which button was pressed is ok? Is it better to use switch..case.. statement?

public class KennelGUI extends JFrame implements ActionListener{
private String filename; // holds the name of the file
private Kennel kennel; // holds the kennel

int buttonsNumber = 10;
private JButton[] buttons = new JButton[buttonsNumber];

private KennelGUI() {
super("Kennel with GUI");

do{
filename = JOptionPane.showInputDialog("Please enter the filename of kennel information: ");
}while(filename==null || filename.isEmpty()); //not good

kennel = new Kennel();

setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
setLocation(300,100);
setSize(200,500);
setVisible(true);
setLayout(new FlowLayout());

buttons[3] = new JButton("Set capacity");
buttons[4] = new JButton("Print dogs with bones");
buttons[5] = new JButton("Print all animals");
buttons[6] = new JButton("Remove dog");
buttons[7] = new JButton("Set kennel name");
buttons[8] = new JButton("Search for pet");
buttons[9] = new JButton("Quit");

for(int i=0;i<buttonsNumber;i++)
{
buttons[i].setPreferredSize(new Dimension(190, 30));
}
}

/*
* initialise() method runs from the main and reads from a file
*/
private void initialise() {
JOptionPane.showMessageDialog(null, "Using file " + filename);

Scanner infile = new Scanner(br)){
Animal newAnimal = null;

String kennelName = infile.nextLine();
kennel.setName(kennelName);

int kennelSize = infile.nextInt();
infile.nextLine();
kennel.setCapacity(kennelSize);

int numPets = infile.nextInt();
infile.nextLine();

for(int i=0; i < numPets; i++){
String type = infile.nextLine();
switch(type)
{
case "Dog":
newAnimal = new Dog();
break;
case "Cat":
newAnimal = new Cat();
break;
case "Monkey":
newAnimal = new Monkey();
break;
default:
throw new IOException("Type problem");//GUI?
}
}

} catch (FileNotFoundException e) {
JOptionPane.showMessageDialog(null,"The file: " + filename + " does not exist.\nAssuming first use and an empty file.\n" +
"If this is not the first use then have you \naccidentally deleted the file?","Warning",JOptionPane.WARNING_MESSAGE);
} catch (IOException e) {
JOptionPane.showMessageDialog(null,"An unexpected error occurred when trying to open the file " + filename,"Warning",JOptionPane.WARNING_MESSAGE);
}
}

@Override
public void actionPerformed(ActionEvent e) {

switch(e.getSource()){
case buttons[0]:
break;
case buttons[1]:
break;
case buttons[2]:
break;
case buttons[3]:
setKennelCapacity();
break;
case buttons[4]:
printDogsWithBones();
break;
case buttons[5]:
printAll();
break;
case buttons[6]:
removePet();
break;
case buttons[7]:
changeKennelName();
break;
case buttons[8]:
searchForPet();
break;
case buttons[9]:
save();
System.exit(0);
break;
}

}
}

• Ok, sorry I am new. Is it better now? I pasted fixed code (changed if..else into switch..case) Apr 5 '15 at 10:40
• Thank you. Now we can see that some of the buttons are related to each other (add dog, add cat, add monkey), which may affect the advice. Apr 5 '15 at 10:41
• I had similar question some time ago, however in my case, I used one big inner class, not method as you. I got advice to split actionPerformed into several classes. You can check for more here: codereview.stackexchange.com/questions/84887/… Apr 16 '15 at 11:52

I'm really not sure if this switch statement works at all. You're comparing objects with it, so your compiler should state that switch only takes integer (or since java 7 string) values.

Normally you wouldn't use e.getSource() and use e.getActionCommand() instead. This allows you to decouple the button objects from their behaviour through using string constants as their action commands.

If you don't set an actionCommand at creation time, you may set it later at the buttons through button[i].setActionCommand(...). If you don't set an action command at all, its default value is the text you give to the button for displaying.

First you should put your possible actions into string constants:

public final static String ADD_DOG_ACTION = "ADD DOG";


then use these constants to set the action command at your buttons:

button[0].setActionCommand(ADD_DOG_ACTION);


and last use these constants in your switch:

switch(e.getActionCommand()){