Implementation of Hangman in Java

Question

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();
 }
 }
 }
}

Answer 1

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.

• Calculus5000: 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?
• kraskevich: @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.