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();
}
}
1 Answer 1
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);
}
}
-
\$\begingroup\$ I couldn't have wished for a better answer! Very informative and helpful, thank you! \$\endgroup\$user97059– user970592019年03月13日 19:10:59 +00:00Commented Mar 13, 2019 at 19:10
with it's
, it should bewith its
. I'll leave the remaining nitpicks to the actual answers. \$\endgroup\$