How can I generalize these repetitive blocks of code?

142 Views Asked by At

Code with near-identical blocks like this makes me cringe. Plus it adds up to where you have a thousand lines of code where half that would suffice. Surely there is a way to make a loop to make it all happen and not have code that looks so unsophisticated and brainless.

Offhand it seems like to do so would be adding as much code as I seek to reduce: loop to make 5 buttons, array of labels for the buttons, array of backgrounds... maybe more. Even if that turned out to be acceptable, how would I make a loop to handle the listeners? I can't have an array of methods, can I? I guess such a loop it would have to include a switch. Yes? I'd probably do that if I didn't want to seek a better solution. So I'm asking...

What would code look like that would listen to the entire group of buttons and take action based on which one was pressed? To which component would I assign the single listener? And how?

(There's a chance that the answer to that question will make me cringe even more than the repetitive nature of the code, if I realize that I already know how to do so and needn't have even asked in the first place, but I'm asking anyway. I'm at one of those I've-had-it-for-today points where the brain just wants out.)

  private void makeScoremasterBonuses(){
    pnlBonuses = new JPanel(new GridLayout(1, 6));
    pnlBonuses.setSize(6,1);

    JButton t1 = (new JButton("3W"));
    t1.setToolTipText("This is a triple-word cell.");
    t1.setBackground(TRIPLE_WORD);
    t1.setHorizontalAlignment(JButton.CENTER);
    t1.addActionListener(new ActionListener() {
      @Override public void actionPerformed(ActionEvent e) {
        Highlighter.shadeSymmetric(currentCell,TRIPLE_WORD);
      }});

    JButton t2 = (new JButton("3L"));
    t2.setToolTipText("This is a triple-letter cell");
    t2.setBackground(TRIPLE_LETTER);
    t2.setHorizontalAlignment(JButton.CENTER);
    t2.addActionListener(new ActionListener() {
      @Override public void actionPerformed(ActionEvent e) {
        Highlighter.shadeSymmetric(currentCell,TRIPLE_LETTER);
      }});

    JButton t3 = (new JButton("2W"));
    t3.setToolTipText("This is a double-word cell");
    t3.setBackground(DOUBLE_WORD);
    t3.setHorizontalAlignment(JButton.CENTER);
    t3.addActionListener(new ActionListener() {
      @Override public void actionPerformed(ActionEvent e) {
        Highlighter.shadeSymmetric(currentCell,DOUBLE_WORD);
      }});

    JButton t4 = (new JButton("2L"));
    t4.setToolTipText("This is a double-letter cell");
    t4.setBackground(DOUBLE_LETTER);
    t4.setHorizontalAlignment(JButton.CENTER);
    t4.addActionListener(new ActionListener() {
      @Override public void actionPerformed(ActionEvent e) {
        Highlighter.shadeSymmetric(currentCell,DOUBLE_LETTER);
      }});

    JButton t5 = (new JButton(""));
    t5.setToolTipText("No bonus");
    t5.setBackground(WHITE);
    t5.setHorizontalAlignment(JButton.CENTER);
    t5.addActionListener(new ActionListener() {
      @Override public void actionPerformed(ActionEvent e) {
        Highlighter.shadeSymmetric(currentCell,B_NORMAL);
      }});

    pnlBonuses.add(new JLabel("Legend: "));
    pnlBonuses.add(t1);    
    pnlBonuses.add(t2);     
    pnlBonuses.add(t3);     
    pnlBonuses.add(t4);     
    pnlBonuses.add(t5);

  }

I'm not asking anyone to write the code; I wouldn't even want that (but I couldn't ignore it!).

Here's what the code above does: enter image description here

5

There are 5 best solutions below

2
On BEST ANSWER

Generally any time you have repeated functionality like that, you want to extract that code out into a helper method like this:

private JButton makeJButton(String label, String toolTip, Color bgColor, final Color highlight) {
    JButton button = new JButton(label);
    button.setToolTipText(toolTip);
    button.setBackground(bgColor);
    button.setHorizontalAlignment(JButton.CENTER);
    button.addActionListener(new ActionListener() {
        @Override
        public void actionPerformed(ActionEvent e) {
            Highlighter.shadeSymmetric(currentCell, highlight);
        }
    });
    return button;
}

Then your makeScoremasterBonuses() method becomes much simpler:

private void makeScoremasterBonuses() {
    pnlBonuses = new JPanel(new GridLayout(1, 6));
    pnlBonuses.setSize(6, 1);

    pnlBonuses.add(new JLabel("Legend: "));
    pnlBonuses.add(makeJButton("3W", "This is a triple-word cell.", TRIPLE_WORD, TRIPLE_WORD));
    pnlBonuses.add(makeJButton("3L", "This is a triple-letter cell.", TRIPLE_LETTER, TRIPLE_LETTER));
    pnlBonuses.add(makeJButton("2W", "This is a double-word cell.", DOUBLE_WORD, DOUBLE_WORD));
    pnlBonuses.add(makeJButton("3L", "This is a double-letter cell.", DOUBLE_LETTER, DOUBLE_LETTER));
    pnlBonuses.add(makeJButton("", "No bonus.", WHITE, B_NORMAL));
}
4
On

An enum could be your friend here. It's almost an array of methods:

static enum Btn {

    TripleWord("3W", "This is a triple word cell.", TRIPLE_WORD),
    TripleLetter("3L", "This is a triple letter cell.", TRIPLE_LETTER),
    DoubleWord("2W", "This is a double word cell.", DOUBLE_WORD),
    DoubleLetter("2L", "This is a double letter cell.", DOUBLE_LETTER),
    NoBonus("", "No bonus.", WHITE, B_NORMAL);
    final String label;
    final String tooltip;
    final Color color;
    final Color shade;

    Btn(String label, String tooltip, Color color, Color shade) {
        this.label = label;
        this.tooltip = tooltip;
        this.color = color;
        this.shade = shade;
    }

    Btn(String label, String tooltip, Color color) {
        this(label, tooltip, color, color);
    }

    public JButton asJButton() {
        JButton btn = (new JButton(label));
        btn.setToolTipText(tooltip);
        btn.setBackground(color);
        btn.setHorizontalAlignment(JButton.CENTER);
        btn.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                Highlighter.shadeSymmetric(currentCell, shade);
            }
        });
        return btn;
    }
}

private void makeScoremasterBonuses() {
    int nBtns = Btn.values().length;
    JPanel pnlBonuses = new JPanel(new GridLayout(1, nBtns + 1));
    pnlBonuses.setSize(nBtns + 1, 1);
    pnlBonuses.add(new JLabel("Legend: "));
    for (Btn btn : Btn.values()) {
        pnlBonuses.add(btn.asJButton());
    }
}
1
On

Identify the aspects that vary, collect them, and iterate over the collection.

Something like this (untested):

pnlBonuses = new JPanel(new GridLayout(1, 6));
pnlBonuses.setSize(6,1);
pnlBonuses.add(new JLabel("Legend: "));

// Create class "CellInfo" with constructor and getters for desired properties.
CellInfo cellInfos[] = {
  new CellInfo("3W", "This is a triple-word cell.",   TRIPLE_WORD),
  new CellInfo("3L", "This is a triple-letter cell.", TRIPLE_LETTER),
  // ...
};

// Add a button for each item described by the cellInfos.
for (CellInfo cellInfo : cellInfos) {
  Button b = new JButton(cellInfo.getLabel());
  b.setToolTipText(cellInfo.getToolTipText());
  b.setBackground(cellInfo.getBackground());
  b.setHorizontalAlignment(JButton.CENTER);
  b.addActionListener(new ActionListener() {
  @Override public void actionPerformed(ActionEvent e) {
    Highlighter.shadeSymmetric(currentCell, cellInfo.getBackground());
  }});
  pnlBonuses.add(b);
}

Note that you might need to create some "final" variables for placeholders for use in the inner anonymous class but the idea should work.

2
On

=== THIS IS A MAJOR EDIT OF WHAT I POSTED AN HOUR AGO ===

I wanted to see if I could implement my own naive method. Here it is:

public class Game implements ActionListener{

  public Color [] backgrounds = {TRIPLE_WORD, TRIPLE_LETTER, 
                                 DOUBLE_WORD, DOUBLE_LETTER, B_NORMAL};

  private void makeScoremasterBonuses(){
    String[] labels = {"3W", "3L", "2W", "2L", "  "};
    JButton but;

    pnlBonuses = new JPanel();
    pnlBonuses.add(new JLabel("Legend:"));

    for (int i = 0; i < labels.length; i++) {
      char wt = labels[i].charAt(0);
      char tp = labels[i].charAt(1);
      but = new JButton(labels[i]);//("" + i);
      but.setBackground(backgrounds[i]);
      but.setHorizontalAlignment(SwingConstants.CENTER);
      but.setActionCommand("" + i);
      but.addActionListener(this);
      but.setToolTipText("This is a " 
          + (i == labels.length - 1 ? "non-bonus" :
                          (wt == '3' ? "triple" : "double") 
                  + " " + (tp == 'L' ? "letter" : "word")) 
          + " cell.");
      pnlBonuses.add(but);
    }    
  }

  public void actionPerformed(ActionEvent evt) {
    int i = Integer.parseInt(evt.getActionCommand());
    Highlighter.shadeSymmetric(currentCell,backgrounds[i]);
  }

This has NOW (after edits) EVEN MORE SO been the best thread I've initiated, in terms of quality of responses and all that I've learned because of them. THANK YOU ALL.

BUT I STILL haven't managed to appropriately use setActionCommand. Whatever I did to TRY to use it wound up being so much longer code-wise that I gave up and went for the short and easy but inappropriate.

Any thoughts about how to use set... and getActionCommand the right way (i.e., as Actions) without adding a ton of code to do so?

3
On

(I know I could have edited my previous answer, but this one's so different...)

Thanks to @OldCurmudgeon, I have come up with what I think is pretty good.

Here's "proof" (I may just leave each label and tooltip as is):

enter image description here

  public enum Colors {
    TRIPLE_WORD    (255, 220,  50), 
    TRIPLE_LETTER  (255, 255, 150), 
    DOUBLE_WORD    (  0, 255,   0), 
    DOUBLE_LETTER  (214, 245, 214),
    NOT_A_BONUS    (255, 255, 255);

    private final int red, green, blue;

    Colors(int r, int g, int b){
      this.red   = r;
      this.green = g;
      this.blue  = b;
     }

    public java.awt.Color background(Colors c){
      return new java.awt.Color(c.red, c.green, c.blue);
    }
  }

  private void makeScoremasterBonuses(){
    Colors c;
    Colors all   [] = Colors.values();
    String labels[] = new String[all.length];
    String abbrs [] = new String[all.length];

    JButton but;
    pnlBonuses = new JPanel();
    pnlBonuses.add(new JLabel("Legend:"));

    for (int i = 0; i < all.length; i++) {
      labels[i] = all[i].name().replace("_", " ").toLowerCase();
      abbrs [i] = abbreviate(all[i].name());
      c = Colors.values()[i];
      but = new JButton(abbrs[i]);
      but.setToolTipText(labels[i]);
      but.setBackground(c.background(c));
      but.setHorizontalAlignment(javax.swing.SwingConstants.CENTER);
      but.setActionCommand("" + i);
      but.addActionListener(this);
      pnlBonuses.add(but);
    }
  }