7
\$\begingroup\$

Here's an implementation of Hangman that I've written that uses a basic GUI. As I am new to Java, please let me know of any improvements I can make to my coding style. Thanks for your help.

import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.io.*;
import java.util.*;
import java.util.List;
import javax.swing.*;


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

    int GuessesRemaining;

    // GUI
    JFrame f;
    JTextField textField;
    JLabel guessesRemainingLabel;
    JLabel lettersGuessedLabel;
    JLabel secretWordLabel;


    public static void main(String[] args){
        Hangman h = new Hangman();
        h.createAlphabetSet();
        h.readFile("words.txt");
        h.buildGUI();
        h.setUpGame();
        h.drawSecretWord();
    }

    // buildGUI - builds the GUI
    private void buildGUI(){
        f = new JFrame("Hangman");

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

        // Text field for user to type letters in
        textField = new JTextField();
        JButton checkButton = new JButton("Guess");

        // Add listeners to textField & checkButton
        TextListener textListener = new TextListener();
        checkButton.addActionListener(textListener);
        textField.addActionListener(textListener);

        // 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
        f.add(BorderLayout.CENTER, labelPanel);

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

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

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

    // chooseSecretWord - selects a word
    private 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);
        }
    }

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

    // drawLettersGuessed - Outputs the letters guessed
    void drawLettersGuessed(){
        StringBuilder lettersBuilder = new StringBuilder();
        for (Character el : 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
    private void drawSecretWord(){
        StringBuilder word = new StringBuilder();
        for(int i=0; i<lettersRevealed.length; i++){

            if(lettersRevealed[i]){
                String s = 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);
                    }
                }
        );
    }

    // loseSequence - when the the user runs out of guesses
    private void loseSequence(){
        for(int i=0; i<lettersRevealed.length; i++)
            lettersRevealed[i] = true;
        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 void playAgain(String message){
        int ans = JOptionPane.showConfirmDialog(f, message,
                "Play again?", JOptionPane.YES_NO_OPTION, JOptionPane.PLAIN_MESSAGE);

        if(ans == JOptionPane.YES_OPTION) {
            setUpGame();
        }else {
            System.exit(0);
        }
    }

    // readFile - read in wordList
    private String[] readFile(String loc){
        try {
            File f = new File(loc);
            assert f.exists() : "File doesn't exist";

            BufferedReader input = new BufferedReader(new FileReader(f));

            // read in the stuff into an arrayList here
            wordList = input.readLine().split(" ");

            // close the input stream
            input.close();
        }catch(IOException ioException){
            ioException.printStackTrace();
        }
        return wordList;
    }

    // setUpGame - sets up the variables appropriately
    private 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

        drawSecretWord();
        drawLettersGuessed();
        drawGuessesRemaining();
    }

    // updateSecretWord - updates which letters of the secret word have been revealed
    private 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--;
            drawGuessesRemaining();
        }
    }

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




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


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


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

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

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

                // Checks if the user has won or lost
                if (checkIfWon())
                    winSequence();
                else if (GuessesRemaining == 0)
                    loseSequence();
            }
        }
    }
}
\$\endgroup\$
0

1 Answer 1

4
\$\begingroup\$
  1. All GUI modification must be done in the GUI thread. It refers to the creation of components, too. It means that your buildGUI method should be invoked on the EDT thread(you do it properly for modifications).

  2. This method does not close the input if readLine throws an exception.

    try {
        File f = new File(loc);
        assert f.exists() : "File doesn't exist";
    
        BufferedReader input = new BufferedReader(new FileReader(f));
    
        // read in the stuff into an arrayList here
        wordList = input.readLine().split(" ");
    
        // close the input stream
        input.close();
    }catch(IOException ioException){
         ioException.printStackTrace();
    }
    

    You can use a try-with-resources statement to fix it:

    try (
            FileReader reader = new FileReader(loc);
            BufferedReader input = new BufferedReader(reader)) {
        // Read the input here.
    } catch (FileNotFoundException e) {
        // Handle the exception according to the specifications.
    } catch (IOException e) {
        // Handle the exception according to the specifications.
    }
    
  3. If you are using Java 8, you can utilize lambda expressions to make your code more concise:

    SwingUtilities.invokeLater(
        new Runnable(){
            @Override
            public void run(){
                ...
            }
        }
    );
    

    can become

    SwingUtilities.invokeLater(() -> {
        // do something    
    });
    
  4. Variables naming: their names should start with a lower-case letter(it might not be the case for constants, but there are none of them in your code). You have it almost right, except for the GuessesRemaining, which starts with a capital letter. You should also give descriptive names to your variables: h and f, tof are not really good.

  5. Indentation and whitespaces: there should be a whitespace after the opening bracket, before the closing one and around binary operators. For instance,

    for (int i = 0; i < lettersRevealed.length; i++) { 
    

    looks better than

    for(int i=0; i<lettersRevealed.length; i++){
    

    In my opinion, there are also too many blank lines inside methods in your code.

  6. Comments: you should try to write self-documenting code. That is, if you have to write comments inside methods, it usually(but not always) means that the code itself is not clear enough(probably a particular part should have been in a separate method) or the comment is just redundant. At the same time, you should write very detailed comments for all public classes and methods(in particular, they should say what a method does, what each parameter stands for, what exception can be thrown, what it returns).

  7. Design: one class should do one thing. That is, I would make two separate classes here: one for the GUI and the other one for the game logic. Moreover, comments in your code(like //GUI before a bunch of fields) indicate that there are two loosely related group of fields, which makes this class a good candidate for splitting it into two or more separate classes. The same is true for methods: if you have several loosely related blocks of code inside one method, it is a good candidate for making several smaller methods. One method should do one thing.

\$\endgroup\$
2
  • \$\begingroup\$ This has been helpful, thanks! I've been having a little difficulty with the design suggestion though. I'm guessing that the action listener is going to need a lot of redesign for me to separate this code into a logic class & a GUI class, right? Do you have any tips for this, as I'm not sure how to do it without copying over some methods that are not dealing with the GUI directly? \$\endgroup\$ Feb 18, 2015 at 16:02
  • 1
    \$\begingroup\$ @Calculus5000 The main idea for separation is that the GUI class should be able just to render some strings and recieve input. There is no need for it to know their meaning. The game logic class should be able to recieve updates(a new string typed in by user and so on) and return the current state: remaining number of guesses, a string represeneting the letters guessed so far etc. \$\endgroup\$
    – kraskevich
    Feb 18, 2015 at 16:13

Your Answer

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

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