1
\$\begingroup\$

I want to know is it possible to re factor my code in such a way to remove duplication of almost identical statements, bar the variable name and what it is initialized to by using methods. Here is the code in question:

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Container;
import java.awt.GridLayout;

import javax.swing.AbstractButton;
import javax.swing.ImageIcon;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;

@SuppressWarnings("serial")
public class Menu extends JFrame {
  private JButton jbtChoc1 = new JButton("cross.gif");
  private JButton jbtChoc2 = new JButton("nought.gif");
  private JButton jbtChoc3 = new JButton("cross.gif");
  private JButton jbtChoc4 = new JButton("nought.gif");
  private JButton jbtChoc5 = new JButton("cross.gif");
  private JButton jbtChoc6 = new JButton("nought.gif");
  private JLabel foodLabelChoice = new JLabel("Main Dishes");
  private ImageIcon food1Image = new ImageIcon("cross.gif");
  private ImageIcon food2Image = new ImageIcon("nought.gif");
  private ImageIcon food3Image = new ImageIcon("cross.gif");
  private ImageIcon food4Image = new ImageIcon("nought.gif");
  private ImageIcon food5Image = new ImageIcon("cross.gif");
  private ImageIcon food6Image = new ImageIcon("nought.gif");

  /**
   * Constructor for the Menu.
   */

  public Menu() {
    Container cont = getContentPane();
    cont.setLayout(new BorderLayout(5, 5));;
    cont.setBackground(Color.white);

    cont.add(foodLabelChoice, BorderLayout.NORTH);


    JPanel girdSetup = new JPanel(new GridLayout(2, 3, 5, 5));
    jbtChoc1.setIcon(food1Image);
    girdSetup.add(jbtChoc1);
    jbtChoc1.setVerticalTextPosition(AbstractButton.BOTTOM);
    jbtChoc1.setHorizontalTextPosition(AbstractButton.CENTER);

    jbtChoc2.setIcon(food2Image);
    girdSetup.add(jbtChoc2);
    jbtChoc2.setVerticalTextPosition(AbstractButton.BOTTOM);
    jbtChoc2.setHorizontalTextPosition(AbstractButton.CENTER);

    jbtChoc3.setIcon(food3Image);
    girdSetup.add(jbtChoc3);
    jbtChoc3.setVerticalTextPosition(AbstractButton.BOTTOM);
    jbtChoc3.setHorizontalTextPosition(AbstractButton.CENTER);

    jbtChoc4.setIcon(food4Image);
    girdSetup.add(jbtChoc4);
    jbtChoc4.setVerticalTextPosition(AbstractButton.BOTTOM);
    jbtChoc4.setHorizontalTextPosition(AbstractButton.CENTER);

    jbtChoc5.setIcon(food5Image);
    girdSetup.add(jbtChoc5);
    jbtChoc5.setVerticalTextPosition(AbstractButton.BOTTOM);
    jbtChoc5.setHorizontalTextPosition(AbstractButton.CENTER);

    jbtChoc6.setIcon(food6Image);
    girdSetup.add(jbtChoc6);
    jbtChoc6.setVerticalTextPosition(AbstractButton.BOTTOM);
    jbtChoc6.setHorizontalTextPosition(AbstractButton.CENTER);

    cont.add(girdSetup, BorderLayout.CENTER);
  }

  /**
   * Main method for test.
   * 
   * @param args Initial setup.
   */

  public static void main(String[] args) {
    Menu frame = new Menu();
    frame.setTitle("Menu");
    frame.setSize(950, 400);
    frame.setVisible(true);
    frame.setLocationRelativeTo(null);
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

  }
} 

I want to remove the duplication of the JButton and ImageIcon declaration and their settings within the constructor so that I simply call a single method instead of copy/paste of code.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

The main duplication I see is:

jbtChoc1.setIcon(food1Image);
girdSetup.add(jbtChoc1);
jbtChoc1.setVerticalTextPosition(AbstractButton.BOTTOM);
jbtChoc1.setHorizontalTextPosition(AbstractButton.CENTER);

So lets make a set of arrays:

ImageIcon[] fooImages = new ImageIcon[6];
JButton[] jbtChoc = new JButton[6];

We fill them using a string array:

private static final String[] names = {"cross.gif", "nought.gif", 
                                       "cross.gif", "nought.gif", 
                                       "cross.gif", "nought.gif"};

for(int i = 0; i < names.length; i++){
    jbtChoc[i] = new JButton(names[i]);
    fooImages[i] = new ImageIcon(names[i]);
}

Then you can call the original chunk of code in a loop as well:

JPanel girdSetup = new JPanel(new GridLayout(2, 3, 5, 5));

for(int i = 0; i<food1Image.length;i++){
    jbtChoc[i].setIcon(foodImage[i]);
    girdSetup.add(jbtChoc[i]);
    jbtChoc[i].setVerticalTextPosition(AbstractButton.BOTTOM);
    jbtChoc[i].setHorizontalTextPosition(AbstractButton.CENTER);
}

cont.add(girdSetup, BorderLayout.CENTER);
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.