pretty much completed hangman, but "new" button is not working

I have almost completed my hangman project. However, I have a JButton called bNew and I attached an ActionListener to it that resets all of the variables to 0, redoes all of the JPanels, and redraws everything. For some reason though the only thing that changes is one of the JLabels and everything else remains the same. The previous word remains shown, and the alphabet remains colored (I make each chosen letter green). Also the KeyListener doesn't work for games after the first either. I can't figure out why this is happening because I am renewing the variables/revalidating the JPanel.

Here's my code for the client. I believe the probem lies in the ActionListener for the bNew button, or in the createBoard method, or both.

Code:

---

import javax.swing.*;
import javax.swing.event.*;
import java.awt.*;
import java.awt.event.*;
import java.util.*;

public class Hangman {
  public static void main(String[] args) {
    Hangman hangman = new Hangman();
  }
 
  JFrame frame = new JFrame();
  JLabel winLabel;
  JButton bNew;
  JPanel south1;
  JPanel south2;
  JPanel south3;
  JPanel south4;
  JPanel south;
  JLabel[] wordArray;
  //JLabel[] abcArray;
  Map<String, JLabel> abcMap;
  Font font;
  HangmanPanel hPanel;
  boolean gameOver = false;
  String gameWord;
 
  private int tries = 0;
  private int wrong = 0;
 
  public Hangman() {
    hPanel = new HangmanPanel();
    hPanel.setPreferredSize(new Dimension(400, 350));
    abcMap = new HashMap<String, JLabel>();
    font = new Font("Comic Sans", Font.BOLD, 18);
    winLabel = new JLabel("");
 
    bNew = new JButton("New");
   
    bNew.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent e) {
        tries = 0;
        wrong = 0;
        gameOver = false;
        hPanel.paintMan(0);
        gameWord = getWord();
        int length = gameWord.length();
        wordArray = new JLabel[length];
        abcMap = new HashMap<String, JLabel>();
        south1 = new JPanel(new FlowLayout());
        south3 = new JPanel(new FlowLayout());
        createBoard(gameWord, length);
      }
    });
   
    south1 = new JPanel(new FlowLayout());
    south1.setForeground(Color.WHITE);
    south1.setBackground(Color.WHITE);
   
    south2 = new JPanel(new FlowLayout());
    south2.setBackground(Color.WHITE);
    south2.add(winLabel);
   
    south3 = new JPanel(new FlowLayout());
    south3.setBackground(Color.WHITE);
   
    south4 = new JPanel(new FlowLayout());
    south4.setBackground(Color.GRAY);
    south4.add(bNew);
   
    south = new JPanel(new GridLayout(4, 1));
   
    south.add(south1);
    south.add(south2);
    south.add(south3);
    south.add(south4);
   
    frame.setLayout(new BorderLayout());
    frame.setTitle("Hangman");
    frame.setSize(new Dimension(400, 450));
    frame.setLocationRelativeTo(null);
    frame.add(hPanel, BorderLayout.CENTER);
    frame.add(south, BorderLayout.SOUTH);
   
    frame.setFocusable(true);
    frame.addKeyListener(new KeyListener() {
      public void keyTyped(KeyEvent e) {
        String key = "" + e.getKeyChar();
        if(gameOver == true) {
        }
        else if(abcMap.containsKey(key) && abcMap.get(key).getForeground() != Color.GREEN) {
          checkKey(key);
          south.revalidate();
        }
        else {
          winLabel.setText("Invalid choice.");
        }
      }
     
      public void keyPressed(KeyEvent e) {
      }
     
      public void keyReleased(KeyEvent e) {
      }
    });
   
    frame.pack();
   
    frame.setVisible(true);
   
    gameWord = getWord();
    int length = gameWord.length();
    wordArray = new JLabel[length];
    createBoard(gameWord, length);
  }

  private void checkKey(String key) {
    int count = 0;
    ArrayList<Integer> wordIndex = new ArrayList<Integer>();
    wordIndex = search(key);
    abcMap.get(key).setForeground(Color.GREEN);
    if(wordIndex.size() > 0) {
      winLabel.setText("Great job! " + key + " is one of the letters!");
    }
    else {
      wrong++;
      hPanel.paintMan(wrong);
      winLabel.setText("Uh oh...");
    }
    while(count < wordIndex.size()) {
      wordArray[wordIndex.get(count)].setForeground(Color.BLACK);
      count++;
    }
    tries++;
    if(tries - wrong == gameWord.length()) {
      winLabel.setText("You won in " + tries + " tries!");
      gameOver = true;
    }
  }
 
  private ArrayList<Integer> search(String key) {
    ArrayList<Integer> aList = new ArrayList<Integer>();
    for(int i = 0; i < wordArray.length; i++) {
      if(wordArray[i].getText().equals(key)) {
        aList.add(i);
      }
    }
    winLabel.setText("" + aList);
    return aList;
  }
 
  private void createBoard(String gameWord, int length) {
   
    winLabel.setText("Press letter on keypad to guess.");
    winLabel.setFont(font);
   
    for(int x = 0; x < length; x++) {
      wordArray[x] = new JLabel("" + gameWord.charAt(x));
      wordArray[x].setFont(font);
      wordArray[x].setForeground(Color.WHITE);
    }
   
    for(int x = 0; x < 26; x++) {
      abcMap.put("" + (char)(x + 97), new JLabel("" + (char)(x + 97)));
    }
   
    for(int x = 0; x < length; x++) {
      south1.add(wordArray[x]);
    }
   
    for(int x = 0; x < 26; x++) {
      abcMap.get("" + (char)(x + 97)).setFont(font);
      south3.add(abcMap.get("" + (char)(x + 97)));
    }
   
    south.revalidate();
  }

  private String getWord() {
    return JOptionPane.showInputDialog(frame, "Please enter word.");
  }
}

---

Here's the code for the hangman panel:

Code:

---

import javax.swing.*;
import java.awt.*;

public class HangmanPanel extends JPanel {
  private int count;
 
  public void paintComponent(Graphics g) {
    super.paintComponent(g);
    setBackground(Color.WHITE);
    Graphics2D g2 = (Graphics2D)g;
    g2.setStroke(new BasicStroke(3));
    g2.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON);
    g2.drawLine(275, 40, 275, 275);
    g2.drawLine(275, 275, 235, 315);
    g2.drawLine(275, 275, 315, 315);
    g2.drawLine(275, 40, 175, 40);
    g2.drawLine(175, 40, 175, 80);
   
    if(count >= 1) {
      g2.fillOval(145, 80, 60, 60);
    }
    if (count >= 2) {
      g2.drawLine(175, 100, 175, 240);
    }
    if (count >= 3) {
      g2.drawLine(175, 190, 145, 160);
    }
    if (count >= 4) {
      g2.drawLine(175, 190, 205, 160);
    }
    if (count >= 5) {
      g2.drawLine(175, 240, 145, 270);
    }
    if (count >= 6) {
      g2.drawLine(175, 240, 205, 270);
    }
  }
 
  public void paintMan(int count) {
    this.count = count;
    repaint();
  }
}

---

Also - if yall have any general tips on how I can improve my coding, that'd be greatly appreciated.

There are several issues that I see, but most importantly are two problems:

1) If in your bNew action listener you create new JPanels and set your southX variables to refer to them, it will have no effect on the JPanels that are being displayed in the GUI. This is similar to a problem someone else was having here on this forum recently where he was getting null results because two variables that he thought were related aren't.
2) Rather than use a keylistener here, you would do well to look into using key binding. They are much more forgiving when it comes to focus issues.

One consideration is to refactor your Hangman class into several subclasses that could correspond to the various panels, each responsible for its own upkeep, each with a reset() method that can be easily called from the bNew actionlistener.

Thanks. I'll start researching keybinding (I just learned about keylistener this morning so I don't really know how to use it properly) and I'll start breaking it into subclasses. That is a good idea and would definitely simplify things.

Just for clarification though - I thought that if I revalidated the south JPanel then it would update the GUI? I'm confused as to why sometimes when I revalidate/add components, the main GUI is changed and other times it won't.

Also - I feel that I definitely made two completely new arrays/maps for "wordArray" and "abcArray" yet the keylistener just stops working. This is confusing the hell out of me. I'll try the keybinding thing but I'd really like to understand why it's not working right now - I'll look at the API again too but a supplementary explanation would be much appreciated.

My guess as to why it's not working is due to a focus issue. For keylisteners to work, the component with the listener has to have focus. When you do your reset, the JFrame may be losing its focus for some reason (again just a guess).

edit: yes, it's a focus issue. After reset, the button has the focus. the JFrame can get it back via:

Code:

---

frame.requestFocus();

---

placed on the last line of the bNew actionlistener. I again recommend keybinding, but in a pinch you could get keylisteners to work.

Thanks. That worked. Why is it that in this case the JPanels being renewed didn't change the GUI though? I think I could get it to work by having a separate "removeAll" method and then re-add all the panels. I definitely understand that breaking it into more separate classes would make it easier, but I just want to understand this better. So all changes made to JPanels without using set____ methods won't show up on the GUI?

I think that it's all about the subtle but important difference between objects and references to objects.

Realize that your GUI isn't holding a "southPanel" object, it initially holds the JPanel object that is referred to by the southPanel variable. Later you can change which JPanel object that the variable refers to, but it doesn't change the object that is held by the GUI.

For instance, I created a class called LetterLabelArray, that for academic integrity reasons, I won't post on here, but I will post the interface that describes what it does, just to give you an idea how to encapsulate these panels:

Code:

---

import java.awt.Color;
import javax.swing.JComponent;
import javax.swing.event.ChangeListener;

public interface LetterLabelable
{
  /**
  * @return the main component to be displayed
  */
  public JComponent getComponent();
 
  /**
  * @param Color c: set the main component's background color
  */
  public void setBackground(Color c); 

  /**
  * add a change listener
  * to respond to mouse clicks on JLabel/Letters that
  * are still "active" (have not been clicked previously).
  * After letter has been clicked, it is no longer active
  * and will not notify listeners if clicked again.
  * @param cListener ChangeListener
  */
  public void addChangeListener(ChangeListener cListener);
 
  /**
  * reset back to initial state
  */
  public void reset();

  /**
  * @return String representing last letter pressed or "" if none yet pressed
  */
  public String getLastLetter();
 
  /**
  * change the text color of the last label pressed
  * @param fg foreground color
  */
  public void setLastLabelForeground(Color fg);
 
  /**
  * @return int how many letters have been pressed
  */
  public int getSelectedLetterCount();


}

---

This panel is dumb, in that it doesn't know what the secret word is. All it does is allows users to choose a letter either by mouse or by keyboard (it results in the very same action, either way), and then notifies all the listeners that a key has been selected.

The key to this class is the addChangeListener method. This allows other objects to add a ChangeListener that notifies them when a key has been selected. The listeners are not told which letter has been selected, only that an active letter (one that has not been pressed before) has been pressed and deactivated. It's up to the classes that are listening to query this class via getLastLetter to find out which letter has been pressed. After a letter has been selected and deactivated, it's text color is set to grey. If the listening object desires, it can then set the text color of the last letter selected to something different (if it is a letter found in the secret word for instance) via the setLastLabelForground method.

When reset, all the letters are made active again, all fonts are changed back to blue.

I see what you did with that code. So just for clarification - I should make one new class to represent the south1, south2, south3, and south panels? And then just add methods to update the appearance and have a method to renew itself?

I'm a little confused as to how it would simplify things though. Even if I did have a renew method for each of the panels, wouldn't I still have to remove and add all of them from the parent panels because the parent panel would be referring to something else, right? I thought that was why my program didn't work before. You said that "it initially holds the JPanel object that is referred to by the southPanel variable. Later you can change which JPanel object that the variable refers to, but it doesn't change the object that is held by the GUI." How would the new class fix this problem? Even though I didn't use that method this particular time, I'd really like to learn how.

I went ahead and finished the bNew button another way. Probably not even close to the most efficient but here it is:

Code:

---

bNew.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent e) {
        tries = 0;
        wrong = 0;
        dupes = 0;
        gameOver = false;
        hPanel.paintMan(0);
        gameWord = getWord();
        int length = gameWord.length();
        wordArray = new JLabel[length];
        abcMap = new HashMap<String, JLabel>();
        south1.removeAll();
        winLabel.setText("Please select letter.");
        south3.removeAll();
        createBoard(gameWord, length);
        south.removeAll();
        south.add(south1);
        south.add(south2);
        south.add(south3);
        south.add(south4);
        frame.repaint();
        frame.remove(south);
        frame.add(south, BorderLayout.SOUTH);
        //THIS LINE BELOW DID IT!
        south.updateUI();
        frame.requestFocus();
      }
    });

---

bump .

I find it easier to not remove or replace panels but instead to keep the same panel and just update the display. It's not the panels that matter, it's the data they display that's key.

Gotcha. Edited code accordingly. Here is my new code:

Code:

---

bNew.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent e) {
        tries = 0;
        wrong = 0;
        dupes = 0;
        gameOver = false;
        hPanel.paintMan(0);
        gameWord = getWord();
        int length = gameWord.length();
        wordArray = new JLabel[length];
        abcMap = new HashMap<String, JLabel>();
        south1.removeAll();
        winLabel.setText("Please select letter.");
        south3.removeAll();
        createBoard(gameWord, length);
        frame.requestFocus();
      }
    });

---

You said earlier that I should divide my program into more classes - could you explain what you meant by this? I know the purpose was to make the renewal easier but how exactly would I do this efficiently? Would I make one interface to represent the different JPanels that need updating? I feel like I would run into a lot of problems with variable visibility between the client and the classes if I incorporated lots of methods into it - particular those involving the array/map.