3
\$\begingroup\$

I developed a simple and short java game in which a random word is selected and the user needs to guess it's letters one after another.

I'm a java beginner so I'll be glad to hear your thoughts.

WordsBank:

import java.util.Random;

public class WordsBank {
    private final String[] Words;

    // Ctor
    public WordsBank(){
        Words = new String[]{ "Adventure", "Hungary", "Pizza", "Madrid", "Flower", "Chicken",
                "Israel", "Romania", "Denmark", "Australia" };
    }

    // Returns a random word from the existing words
    public String getRandomWord() {
        Random rand = new Random();
        return Words[rand.nextInt(Words.length)]; // Word from a random index
    }
}

ChosenWord:

public class ChosenWord {

    private String word; // The chosen word
    private boolean[] charsGuessed;  // Array of the guessed chars in the word.
    // If charsGuessed[0] is True,
    // it means the first char was guessed by the user already
    // Initially, all the values are False

    // Ctor
    public ChosenWord(String word){
        this.word = word.toLowerCase();
        charsGuessed = new boolean[word.length()];
    }


    // Check if the entire word is already guessed
    public boolean isEntireWordGuessed() {

        // Iterating through the chars guessed array
        for (boolean b : charsGuessed) {
            // If a char was not guessed returning false
            if (!b)
                return false;
        }

        // All the chars were guessed, return true.
        return true;
    }


    // receives a char and checks if it appears in the word.
    public void charGuess(char guess) {
        int index = word.indexOf(guess); // Finding first occurrence
        // Iterating while there are more occurrences of the guess
        while (index >= 0) {
            charsGuessed[index] = true; // Marking the char appearance in the fitting index
            index = word.indexOf(guess, index + 1); // Finding next occurrence
        }
    }

    // Building a string to represent the chosen word with it's revealed letters
    @Override
    public String toString(){
        StringBuilder formattedWord = new StringBuilder();

        // Iterating through the characters of the word. if the character was guessed, adding it, otherwise add '_'
        for(int index = 0; index < word.length(); index++){
            if (charsGuessed[index]){
                formattedWord.append(word.charAt(index));
            } else {
                formattedWord.append('_');
            }

            formattedWord.append(' ');
        }

        return formattedWord.toString();
    }
}

Game:

import javax.swing.*;

public class Game {

    private int numberOfGuesses;
    private String unguessedCharacters;
    private ChosenWord chosenWord;
    private WordsBank wordsBank = new WordsBank();
    private JFrame frame = new JFrame("Input");


    public void startNewGame(){

        // The abc letters to guess
        this.unguessedCharacters = "abcdefghijklmnopqrstuvwxyz";

        numberOfGuesses = 0;

        // Getting a new random word to guess
        this.chosenWord = new ChosenWord(wordsBank.getRandomWord());

        inputUserLetterGuess();
    }

    // Handling a guess from the user, guessedChar is the guessed char
    private void handleUserLetterGuess(char guessedChar){

        // Increasing number of guesses
        numberOfGuesses++;

        // Removing the guessed letter, so that the user can't guess it again
        removeOptionalCharGuess(guessedChar);

        // Running the guessing logic
        chosenWord.charGuess(guessedChar);
    }

    private void removeOptionalCharGuess(char guessedChar){
        // Replacing the guessed char with empty char, so it can no longer be guessed
        unguessedCharacters = unguessedCharacters.replace(Character.toString(guessedChar), "");
    }

    private void inputUserLetterGuess() {
        Character[] charactersArray = new Character[unguessedCharacters.length()];

        // Converting to Characters array to be able to present in the dialog
        for (int index = 0; index < charactersArray.length; index++) {
            charactersArray[index] = Character.valueOf(unguessedCharacters.charAt(index));
        }

        // Input dialog
        Character guessedLetter = (Character) JOptionPane.showInputDialog(frame,
                "What letter do you want to guess?",
                "Letter guess",
                JOptionPane.QUESTION_MESSAGE,
                null,
                charactersArray,
                charactersArray[0]);

        // Validating return value
        if (guessedLetter == null){
            exit();
            return;
        }


        // Handling the user guess
        handleUserLetterGuess(guessedLetter);

        // Display results of the guess
        displayUserGuessResults(frame);
    }

    // Displays the results of the guess to the user
    private void displayUserGuessResults(JFrame frame){
        // Displaying result

        JLabel wordStartLabel = new JLabel("After your guess: " + chosenWord.toString());
        JButton button = new JButton();

        JPanel panel = new JPanel();
        panel.add(wordStartLabel);
        panel.add(button);

        frame.add(panel);
        frame.setSize(300, 300);
        frame.setLocationRelativeTo(null);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setVisible(true);

        // Checking if the word is completely exposed
        if (!chosenWord.isEntireWordGuessed()){
            // If it isn't continue guessing
            button.addActionListener(e -> {
                frame.remove(panel);
                inputUserLetterGuess();
            });
            button.setText("Continue guessing");
        } else {
            JLabel guessesLabel = new JLabel("Congratulations! number of guesses is: " + numberOfGuesses);
            panel.add(guessesLabel);
            // If it is, show result and give option to start a new game
            button.addActionListener(e -> {
                frame.remove(panel);
                startNewGame();
            });
            button.setText("Start a new game");
        }
    }

    // Closing the frame on forced exit
    private void exit() {
        frame.dispose();
    }
}

Main

public class Main {

    public static void main(String[] args)
    {
        // Starts a new game
        Game game = new Game();
        game.startNewGame();
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ I just scanned through the whole program and it looks really nice. The many comments in the first two classes are necessary right now. Soon, when you have gained more experience, you will be able to read the code without them. Instead of with it's, it should be with its. I'll leave the remaining nitpicks to the actual answers. \$\endgroup\$ – Roland Illig Mar 12 at 7:29
  • \$\begingroup\$ Thanks, I'm always happy to learn English Grammer too and not just coding ;) \$\endgroup\$ – user97059 Mar 12 at 8:25
3
\$\begingroup\$

Swing Event Dispatching Thread

Always create and manipulate UI elements on Swing's EDT. It isn't hurting you yet, but it may later. It is better to get into the habit now:

public static void main(String[] args) {
    SwingUtilities.invokeLater(new Runnable() {
        @Override
        void run() {
            Game game = new Game();
            game.startNewGame();
        }
    });
}

If Java8's lambda syntax doesn't frighten you, you could use:

public static void main(String[] args) {
    SwingUtilities.invokeLater( () -> { Game game = new Game(); game.startNewGame(); } );
}

Or even:

public static void main(String[] args) {
    SwingUtilities.invokeLater( () -> new Game().startNewGame() );
}

Word Bank

Static Strings

Instead of setting the strings in the constructor, you could initialize them at declaration.

private final String[] words = { "Adventure", "Hungary", ... "Australia" };

Since the strings do no change, and every instance of WordsBank will have the same strings, this should even be static:

private final static String[] words = { "Adventure", "Hungary", ... "Australia" };

Random

You are creating a new Random instance every time you call getRandomWord(). This is a little expensive. You could create the Random instance once, save it as a member:

private Random rand = new Random();

public String getRandomWord() {
    return Words[rand.nextInt(Words.length)];
}

Or use a common thread-local random number generator:

public String getRandomWord() {
    return Words[ThreadLocalRandom.current().nextInt(Words.length)];
}

Game

this.

You are not consistent in your usages of this.. For example:

this.unguessedCharacters = "abcdefghijklmnopqrstuvwxyz";

numberOfGuesses = 0;

this.chosenWord = new ChosenWord(wordsBank.getRandomWord());

Either use this. everywhere you are referencing an instance member:

this.numberOfGuesses = 0;

this.chosenWord = new ChosenWord(this.wordsBank.getRandomWord());

Or (my preference) don't use this. at all:

unguessedCharacters = "abcdefghijklmnopqrstuvwxyz";

chosenWord = new ChosenWord(wordsBank.getRandomWord());

frame

The member function inputUserLetterGuess() has access to the instance member frame, and so can make the call displayUserGuessResults(frame);

But displayUserGuessResults() is also a member function, and has access to frame. You don't need to pass it in as an argument.

displayUserGuessResults()

Ok - here is where your program really is in need of work. You are recreating the GUI each time the user guesses a letter, and then destroying it if the user selects "Continue Guessing". This is not the way to write a Swing GUI.

Your game should have a JFrame, with one or more JPanel inside. In one JPanel, you should have a JLabel with the letter blanked word displayed. Your Game object should hold onto that JLabel as an instance variable, and after each guess is made, update the text of that JLabel to reveal the user's progress.


Example Implementation with static GUI. I have all code entirely within one file, so only the Main class is declared public. Feel free to split into multiple files and declare the other classes public as well, if desired. The utility classes are very close to how you wrote them, with minor tweaks from my code review comments above. The Game class is significantly reworked, because I dislike the JOptionsPane being used for user input. Instead, I've added 26 button to the UI, and disable buttons as they have been pressed to prevent the user guessing those letters again. The layout is ugly, but it was a quick and dirty implementation, as an example of how to create a UI once at the beginning of the application and update it as the game progresses, instead of recreating the UI at each and every point. Hope it helps.

import java.awt.BorderLayout;
import java.awt.GridLayout;
import java.awt.event.ActionEvent;
import java.util.ArrayList;
import java.util.Random;

import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.SwingUtilities;

class WordsBank {
    private final static String[] words = { "Adventure", "Hungary", "Pizza", "Madrid", "Flower",
            "Chicken", "Israel", "Romania", "Denmark", "Australia" };

    private final Random rand = new Random();

    public String getRandomWord() {
        return words[rand.nextInt(words.length)];
    }
}

class ChosenWord {

    private String word;
    private boolean[] chars_guessed; 

    public ChosenWord(String word){
        this.word = word.toLowerCase();
        chars_guessed = new boolean[word.length()];
    }

    public boolean isEntireWordGuessed() {

        for (boolean b : chars_guessed) {
            if (!b)
                return false;
        }

        return true;
    }

    public void charGuess(char guess) {
        int index = word.indexOf(guess);
        while (index >= 0) {
            chars_guessed[index] = true; 
            index = word.indexOf(guess, index + 1);
        }
    }

    @Override
    public String toString(){
        StringBuilder formatted_word = new StringBuilder();

        for(int index = 0; index < word.length(); index++) {
            if (chars_guessed[index]) {
                formatted_word.append(word.charAt(index));
            } else {
                formatted_word.append('_');
            }

            formatted_word.append(' ');
        }

        return formatted_word.toString();
    }
}

class Game {

    private final static String ALL_LETTERS = "abcdefghijklmnopqrstuvwxyz";

    private final WordsBank words_bank = new WordsBank();
    private final JFrame frame = new JFrame("Guess the Word");
    private final JLabel puzzle_word;
    private final ArrayList<JButton> letter_buttons = new ArrayList<>();

    private int number_guesses;
    private ChosenWord chosen_word;

    Game() {
        frame.setSize(300, 300);
        frame.setLocationRelativeTo(null);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        JPanel panel = new JPanel(new BorderLayout());
        puzzle_word = new JLabel("Puzzle: ");
        panel.add(puzzle_word, BorderLayout.PAGE_START);

        JPanel grid = new JPanel(new GridLayout(0, 7));
        for (int i=0; i<ALL_LETTERS.length(); i++) {
            String letter = ALL_LETTERS.substring(i, i+1);
            JButton btn = new JButton(letter);
            btn.setActionCommand(letter);
            btn.addActionListener(this::guessLetter);
            letter_buttons.add(btn);
            grid.add(btn);
        }
        panel.add(grid, BorderLayout.CENTER);

        JButton btn = new JButton("Start a new Game");
        panel.add(btn, BorderLayout.PAGE_END);
        btn.addActionListener(ActionEvent -> this.reset());

        frame.setContentPane(panel);

        reset();
        frame.setVisible(true);
    }

    private void reset() {
        chosen_word = new ChosenWord(words_bank.getRandomWord());
        number_guesses = 0;

        for(JButton btn : letter_buttons) {
            btn.setEnabled(true);
        }

        update_game_state();
     }

    private void guessLetter(ActionEvent evt) {
        char guessed_letter = evt.getActionCommand().charAt(0);
        handleUserLetterGuess(guessed_letter);

        JButton button = (JButton) evt.getSource();
        button.setEnabled(false);

        if (chosen_word.isEntireWordGuessed()) {
            for (JButton btn : letter_buttons) {
                btn.setEnabled(false);
            }
        }
    }

    private void handleUserLetterGuess(char guessed_char){

        number_guesses++;
        chosen_word.charGuess(guessed_char);
        update_game_state();
    }

    private void update_game_state() {
        puzzle_word.setText("Puzzle: " + chosen_word + ", guesses: "+ number_guesses);      
    }
}

public class Main {

    public static void main(String[] args) {
        SwingUtilities.invokeLater(Game::new);
    }
}
\$\endgroup\$
  • \$\begingroup\$ I couldn't have wished for a better answer! Very informative and helpful, thank you! \$\endgroup\$ – user97059 Mar 13 at 19:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy