Hangman program in Java

Question

Here's an implementation of Hangman using Java 6. I've split the code into 2 classes - a logic class & a gui class. Is it ok to have so many static member variables? Please let me know if there are any design improvements that I could make.

Hangman logic - main class

    import java.awt.*;
    import java.io.*;
    import java.util.*;
    import java.util.List;
    import javax.swing.*;


    public class Hangman {
        static String[] wordList;
        static String secretWord;
        static Set<Character> alphabet;
        static Set<Character> lettersGuessed;    // letters the user has guessed
        static boolean[] lettersRevealed;       // determines if the letter should be revealed or not
        static int guessesRemaining;



        public static void main(String[] args){
            Hangman hangman = new Hangman();
            hangman.createAlphabetSet();
            hangman.readFile("words.txt");

            HangmanGUI.buildGUI();
            setUpGame();
        }



        // checkIfWon - sees if the user has won the game
        static boolean checkIfWon(){
            for(boolean isLetterShown : lettersRevealed){
                if(!isLetterShown)
                    return false;
            }
            return true;
        }

        // checkUserGuess - get input from the user
        static boolean checkUserGuess(String l){
            if(l.length() == 1 && !lettersGuessed.contains(l.charAt(0)) && alphabet.contains(l.charAt(0))) {
                HangmanGUI.setText(null);
                lettersGuessed.add(l.charAt(0));
                return true;
            }else{
                Toolkit.getDefaultToolkit().beep();
            }
            return false;
        }

        // chooseSecretWord - selects a word
        private static String chooseSecretWord(String[] wordList){
            return wordList[(int)(Math.random() * wordList.length)];
        }

        // createAlphabetSet - Creates the alphabet set that's used to ensure that the user's guess not a number nor a special character
        private void createAlphabetSet(){
            alphabet = new HashSet<Character>(26);
            for(Character c = 'a'; c<='z'; c++){
                alphabet.add(c);
            }
        }

        // loseSequence - when the the user runs out of guesses
        static void loseSequence(){
            for(int i = 0; i < lettersRevealed.length; i++){
                lettersRevealed[i] = true;
            }
            HangmanGUI.drawSecretWord();
            playAgain("Tough luck. The secret word was " + secretWord + ".\nWould you like to play another game of hangman?");
        }

        // playAgain - Allows the user to play another game of hangman
        private static void playAgain(String message){
            int ans = HangmanGUI.playAgainDialog(message);
            if(ans == JOptionPane.YES_OPTION){
                setUpGame();
            }else{
                System.exit(0);
            }
        }

        // readFile - read in wordList
        private String[] readFile(String loc){
            BufferedReader input = null;
            try{
                input = new BufferedReader(new InputStreamReader(this.getClass().getClassLoader().getResourceAsStream(loc)));
                wordList = input.readLine().split(" ");
            }catch(IOException ioException) {
                ioException.printStackTrace();
            }finally{
                try {
                    if (input != null) input.close();
                }catch(IOException ioEx){
                    ioEx.printStackTrace();
                }
            }
            return wordList;
        }

        // setUpGame - sets up the variables appropriately
        private static void setUpGame(){
            guessesRemaining = 5;
            secretWord = chooseSecretWord(wordList);
            lettersRevealed = new boolean[secretWord.length()];
            Arrays.fill(lettersRevealed, false);
            lettersGuessed = new HashSet<Character>(26);     // 26 letters in alphabet

            HangmanGUI.drawSecretWord();
            HangmanGUI.drawLettersGuessed();
            HangmanGUI.drawGuessesRemaining();
        }

        // updateSecretWord - updates which letters of the secret word have been revealed
        static void updateSecretWord(String l){
            List<Integer> changeBool = new ArrayList<Integer>();

            if(secretWord.contains(l)){
                // Searches through secretWord & notes down all letters that equal the user's guess
                for(int i=0; i<secretWord.length(); i++){
                    if(secretWord.charAt(i) == l.charAt(0))
                        changeBool.add(i);
                }

                // Changes the boolean value for those letters @ their corresponding indexes
                for(Integer idx : changeBool)
                    lettersRevealed[idx] = true;
            }else{
                guessesRemaining--;
                HangmanGUI.drawGuessesRemaining();
            }
        }

        // winSequence - when the user has correctly guessed the secret word
        static void winSequence(){
            playAgain("Well done! You guessed " + secretWord + " with " + guessesRemaining + " guesses left!\n" +
                    "Would you like to play another game of hangman?");
        }

    }

Hangman GUI

    import java.awt.*;
    import java.awt.event.ActionEvent;
    import java.awt.event.ActionListener;
    import javax.swing.*;


    public class HangmanGUI {

        // GUI
        static JFrame frame;
        static JTextField textField;
        static JLabel guessesRemainingLabel;
        static JLabel lettersGuessedLabel;
        static JLabel secretWordLabel;

        // buildGUI - builds the GUI
        static void buildGUI(){
            SwingUtilities.invokeLater(
                    new Runnable(){
                        @Override
                        public void run(){
                            frame = new JFrame("Hangman");

                            // JLabels
                            guessesRemainingLabel = new JLabel("Guesses remaining: " + String.valueOf(Hangman.guessesRemaining));
                            lettersGuessedLabel = new JLabel("Already guessed: ");
                            secretWordLabel = new JLabel();

                            // TextField & checkButton
                            textField = new JTextField();
                            JButton checkButton = new JButton("Guess");
                            GuessListener guessListener = new GuessListener();
                            checkButton.addActionListener(guessListener);
                            textField.addActionListener(guessListener);

                            // Panel for all the labels
                            JPanel labelPanel = new JPanel();
                            labelPanel.setLayout(new BoxLayout(labelPanel, BoxLayout.PAGE_AXIS));
                            labelPanel.add(guessesRemainingLabel);
                            labelPanel.add(lettersGuessedLabel);
                            labelPanel.add(secretWordLabel);

                            // User panel
                            JPanel userPanel = new JPanel(new BorderLayout());
                            userPanel.add(BorderLayout.CENTER, textField);
                            userPanel.add(BorderLayout.EAST, checkButton);
                            labelPanel.add(userPanel);

                            // Add everything to frame
                            frame.add(BorderLayout.CENTER, labelPanel);

                            frame.setSize(250, 100);
                            frame.setLocationRelativeTo(null);
                            frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
                            frame.setVisible(true);
                        }
                    }
            );
        }

        // drawGuessesRemaining - Outputs the guesses remaining
        static void drawGuessesRemaining(){
            final String guessesMessage = "Guesses remaining: " + String.valueOf(Hangman.guessesRemaining);
            SwingUtilities.invokeLater(
                    new Runnable(){
                        @Override
                        public void run(){
                            guessesRemainingLabel.setText(guessesMessage);
                            guessesRemainingLabel.setAlignmentX(Component.LEFT_ALIGNMENT);
                        }
                    }
            );
        }

        // drawLettersGuessed - Outputs the letters guessed
        static void drawLettersGuessed(){
            StringBuilder lettersBuilder = new StringBuilder();
            for (Character el : Hangman.lettersGuessed) {
                String s = el + " ";
                lettersBuilder.append(s);
            }

            final String letters = lettersBuilder.toString();
            SwingUtilities.invokeLater(
                    new Runnable() {
                        @Override
                        public void run() {
                            lettersGuessedLabel.setText("Already guessed: " + letters);
                        }
                    }
            );
        }

        // drawSecretWord - draws the secret word with dashes & etc for user to use to guess the word with
        static void drawSecretWord(){
            StringBuilder word = new StringBuilder();
            for(int i=0; i<Hangman.lettersRevealed.length; i++){

                if (Hangman.lettersRevealed[i]) {
                    String s = Hangman.secretWord.charAt(i) + " ";
                    word.append(s);
                } else {
                    word.append("_ ");
                }
            }

            final String w = word.toString();
            SwingUtilities.invokeLater(
                    new Runnable(){
                        @Override
                        public void run() {
                            secretWordLabel.setText(w);
                        }
                    }
            );
        }

        //playAgainDialog - shows the confirm w
        static int playAgainDialog(String m){
            return JOptionPane.showConfirmDialog(frame, m, "Play again?", JOptionPane.YES_NO_OPTION, JOptionPane.PLAIN_MESSAGE);
        }


        // GETTERS
        private static String getText(){
            return textField.getText();
        }

        // SETTERS
        static void setText(final String t){
            SwingUtilities.invokeLater(
                    new Runnable() {
                        @Override
                        public void run() {
                            textField.setText(t);
                        }
                    }
            );
        }


        // ActionListener
        private static class GuessListener implements ActionListener {
            @Override
            public void actionPerformed(ActionEvent ev){
                String guess = getText();

                if(Hangman.checkUserGuess(guess)) {
                    Hangman.updateSecretWord(guess);
                    drawSecretWord();

                    if(Hangman.lettersGuessed.size() != 0)      // No letters have been guessed by the user at the beginning
                        drawLettersGuessed();

                    // Checks if the user has won or lost
                    if (Hangman.checkIfWon())
                        Hangman.winSequence();
                    else if (Hangman.guessesRemaining == 0)
                        Hangman.loseSequence();
                }
            }
        }
    }

Answer 1

Is it ok to have so many static member variables?

No, it's almost always a sign of bad design (and not just because of the static variables, but also the static functions; only utility functions should be static). Another bad sign is that you import javax.swing.* in your logic class.

You classes also do too many things, which makes them very static, hard to read, and hard to write automated tests for.

I would at least create:

• aHangmanGame which contains the word that currently has to be guessed, the guessed letters, the revealed letters, and the remaining guesses. Maybe the alphabet set as well. But it doesn't deal with any user input or the like, it just statically stores the game data and handles the logic. The constructor would only take the word to be guessed, and maybe remaining guesses. Methods might be public boolean guess(Character), public boolean isGameOver(), public boolean isGameWon, String[] getWrongGuessesMade() (or create a Guess class, which then can have a field wrong, and return a list of those classes), etc. All these methods should not be static.
• HangmanMain: the main game loop. Build gui, game, etc; manage play again, etc.
• WordReader: gets the words
• HangmanGUI: pretty much as before, but don't let it be responsible for getting the data it needs. drawGuessesRemaining() for example could look like this: drawGuessesRemaining(int guesses) and then used like this in HangmanMain: // init gui somewhere at the beginning: HangmanGUI hangmanGUI = new HangmanGUI(); // somewhere else: hangmanGUI.drawGuessesRemaining(5);
• I would move the GuessListener to its own class.

Misc

• don't import *, but import all classes explicitly, so a reader knows exactly what you use.
• use private (or public if it makes sense) instead of the default package private.
• use curly brackets even for one line statements
• use JavaDoc style comments for more readable method documentation. Also, some comments could be improved (eg chooseSecretWord - selects a word: a word for what? What happens with the selected word?
• don't hardcode magic numbers such as 5, because it makes it hard to see what the 5 stands for, and it makes it hard to reconfigure the game. Move these to static fields. This is especially bad for lettersGuessed = new HashSet<Character>(26), because it is independent of the 26 used in the createAlphabetSet function. So theoretically, I could reconfigure the game to generate a bigger alphabet set (because I want to include ß and ä), raise the guessesRemaining to 28 (because who likes losing?), and then get an exception (a bit of a hypothetical, but I think the idea is clear).
• You don't have to declare the type twice when declaring something. List<Integer> changeBool = new ArrayList<>() works fine.
• some of your variable names are rather short (s, el, idx, ans, etc). This is only in a small context, so it's not that bad, but I would prefer more expressive names
• your spacing is sometimes off