patternjavaMinor
Implementation of Hangman in Java
Viewed 0 times
javaimplementationhangman
Problem
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 alphabet;
Set 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
```
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 alphabet;
Set 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
Solution
-
All GUI modification must be done in the GUI thread. It refers to the creation of components, too. It means that your
-
This method does not close the
You can use a try-with-resources statement to fix it:
-
If you are using Java 8, you can utilize lambda expressions to make your code more concise:
can become
-
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
-
Indentation and whitespaces: there should be a whitespace after the opening bracket, before the closing one and around binary operators. For instance,
looks better than
In my opinion, there are also too many blank lines inside methods in your code.
-
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).
-
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
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).-
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.
}-
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
});-
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. -
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.
-
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).
-
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.Code Snippets
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();
}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.
}SwingUtilities.invokeLater(
new Runnable(){
@Override
public void run(){
...
}
}
);SwingUtilities.invokeLater(() -> {
// do something
});for (int i = 0; i < lettersRevealed.length; i++) {Context
StackExchange Code Review Q#80718, answer score: 4
Revisions (0)
No revisions yet.