Hangman game in Java - second try

Question

Please look over my new object-oriented version of my Hangman game I posted here about two weeks ago.

I know I should work more on commenting and be clearer and more descriptive of the methods and their parameters and returns.

Menu class

public class Menu {
 public static void main(String[] args) {
 //Loop that will run until user chooses an input that causes exit
 while(true) {
 System.out.println();
 Menu menu = new Menu();
 menu.menuChoice(menu.menuSelect());
 }
 }
 // Prints out menu and calls method in UserInput class to get user input for menuselection
 public int menuSelect() {
 UserInput userInput = new UserInput();
 System.out.println("What do you want to do? \n1.Play Hangman\n2.Add a word to the word file\n3.List all the words in the list\n4.Exit");
 return userInput.inputMenuChoice();
 }
 /**
 *Selects the menu option with the Integer
 *
 *@param option -integer value that selects where to go with switch statement, -from menuSelect method
 */
 public void menuChoice(int option){
 FileHandling filehandling = new FileHandling();
 UserInput userInput = new UserInput();
 Word word = new Word();
 Game game = new Game();
 //Selects where to go
 switch(option) {
 case 1: 
 game.gameLoop();
 break;
 case 2: 
 filehandling.writeToFile(userInput.wordToWrite());
 break;
 case 3: 
 word.wordList(filehandling.readFile());
 break;
 case 4: 
 System.exit(0);
 break;
 default:
 System.exit(0);
 break;
 }
 } 
}

Board Class

public class Board {
 //Hangman Board - two dimensional array
 static char[][] board = {
 {'_', '_', '_', '_', '_', '_', '_', '_', '_', '_', '_', '_', '_'},
 {'_', '_', '_', '_', '_', '_', '_', '_', '_', '_', '_', '_', '|'},
 {'|', ' ', '|', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', '|', ' '},
 {'|', ' ', '|', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', '|', ' '},
 {'|', ' ', '|', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '},
 {'|', ' ', '|', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '},
 {'|', ' ', '|', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '},
 {'|', ' ', '|', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '},
 {'|', ' ', '|', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '},
 {'|', ' ', '|', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '},
 {'|', ' ', '|', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '},
 {'|', ' ', '|', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '},
 {'|', ' ', '|', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '},
 {'|', '_', '|', '_', '_', '_', '_', '_', '_', '_', '_', '_', '_'},
 };
 /**
 *Writes to board depending on number of wrong inputs
 *
 *@param mistakes -number of wrong inputs - from game class
 */
 public void writeToBoard(int mistakes) {
 switch(mistakes) 
 {
 case 1: 
 board[4][11] = '0';
 break;
 case 2:
 board[5][11] = '|';
 break;
 case 3: 
 board[6][11] = '|';
 break;
 case 4: 
 board[7][11] = '|';
 break;
 case 5: 
 board[6][12] = '\\';
 break;
 case 6: 
 board[6][10] = '/';
 break;
 case 7: 
 board[8][11] = '\\';
 break;
 case 8: 
 board[8][9] = '/';
 System.out.println("You have lost!");
 break;
 default: break;
 } 
 }
 /**
 *Prints out the board along with used words and used characters in the game
 *
 *@param hangmanWordDisplay[] -char array from game class that displays the number of _ and characters found
 *@param usedCharacters[] -char array filled with used characters guessed by the user, correct and incorrect ones
 *@param usedWords[] - String array filled with used words guessed by the user, all incorrect
 */
 public void printBoard(char[] hangmanWordDisplay, char[] usedCharacters, String[] usedWords) {
 // Prints board
 for(char[] c : board) 
 {
 for(char elem : c) 
 {
 System.out.print(elem);
 }
 System.out.println();
 }
 //Prints out used characters
 System.out.print("\nUsed Characters: ");
 for(char c : usedCharacters) {
 System.out.print(c + "|");
 }
 //Prints out used words
 System.out.print("\nUsed words: ");
 for(String words : usedWords) {
 if(words == null) {
 System.out.print(" ");
 }else {
 System.out.print(words+ "|");
 }
 }
 // Prints hangmanwordDisplay array
 System.out.println();
 for(int i = 0; i < hangmanWordDisplay.length; i++) {
 System.out.print(hangmanWordDisplay[i]);
 } 
 System.out.print("\n");
 }
}

Word Class

public class Word {
 /**
 *Selects a word for the hangman game
 *
 *@param words[] -array of all the words read from the word file
 *@return - returns random word from the word array to be used in the game.
 */
 public String selectWord(String[] words) {
 return words[randWord(words.length)];
 }
 //Math function returning a random number
 public int randWord(int length) {
 return (int)(Math.random() * length);
 }
 // Returns true if the character entered by the user is equal to the character sent from the word in the game class
 public boolean testCharacter(char testChar, char charFromWord) {
 //Calls method to check if the characters are equal ignoring the case
 if(equalsIgnoreCase(testChar, charFromWord)) {
 return true;
 }
 return false;
 }
 // Method that uses .toLowerCase to make sure the if test ignores the case of the characters
 public boolean equalsIgnoreCase(char testChar, char charFromWord) {
 if(Character.toLowerCase(testChar) == Character.toLowerCase(charFromWord)) {
 return true;
 }
 return false;
 }
 //Checks if the word the user entered is equal to the word selected as the hangman word
 public boolean testWord(String testWord, String hangmanWord) {
 if(testWord.equalsIgnoreCase(hangmanWord)) {
 return true;
 }
 return false;
 }
 // Checks the userinput agains previously used characters found in the usedChar array
 public boolean hasUsedChar(char[] usedChar, char testChar) {
 //Iterates through the array
 for(char c : usedChar) {
 if(c == testChar) {
 return true;
 }
 }
 return false;
 }
 //Checks if the word entered had been a previously used word found in the usedWords array
 public boolean hasUsedWord(String[] usedWords, String word) {
 //Iterates through the usedWords array
 for(String Word : usedWords) {
 if(Word == null) { //avoids null pointer exception by telling ignore the rest of the code if the array element is empty
 continue;
 }
 if(Word.equalsIgnoreCase(word)) {
 return true;
 }
 }
 return false;
 }
 // Prints out all the words found in the word array
 public void wordList(String[] words) {
 for(String word : words) {
 System.out.println(word);
 }
 }
}

UserInput Class

import java.util.Scanner;
import java.util.InputMismatchException;
public class UserInput {
 private Scanner input = new Scanner(System.in);
 // Gets the Integer input for the menu class to select menu option
 public int inputMenuChoice() {
 int option = 0;
 try {
 option = input.nextInt();
 }catch(InputMismatchException e) {
 e.printStackTrace();
 }
 return option;
 }
 // Gets input for the hangman game for the game class
 public String gameInput() {
 System.out.print("Enter next character or attempt the full word: ");
 String userInput;
 try {
 userInput = input.nextLine();
 return userInput;
 }catch(InputMismatchException e) {
 e.printStackTrace();
 gameInput();
 }
 return null; 
 }
 // Gets a word from the user that's written to the word file
 public String wordToWrite() {
 System.out.println("Enter the word you wish to use");
 String userInput;
 try {
 userInput = input.nextLine();
 return userInput;
 }catch(InputMismatchException e) {
 e.printStackTrace();
 wordToWrite();
 }
 return null;
 }
}

FileHandling Class

import java.io.FileWriter;
import java.io.File;
import java.io.FileNotFoundException;
import java.util.Scanner;
import java.io.IOException;
public class FileHandling {
 private File file = new File("word.txt");
 Scanner fileRead;
 public String[] wordArray;
 /**Constructor for FileHandling class
 *Checks if the file 'word.txt' already exists and creates it if it doesnt 
 */
 public FileHandling() {
 if(!file.exists()) {
 try {
 file.createNewFile();
 FileWriter writer = new FileWriter(file, true);
 fileRead = new Scanner(file);
 try {
 writer.write("Sindre\n");
 writer.write("Schmidt\n");
 }finally {
 writer.close();
 }
 }catch(IOException e) {
 e.printStackTrace();
 }
 }
 }
 /**
 *Reads all the lines from 'words.txt' in to a string array
 *@returns a string array to the game class 
 */
 public String[] readFile() {
 Scanner input;
 wordArray = new String[fileLength()];
 int count = 0;
 try {
 input = new Scanner(file);
 while(input.hasNextLine()) {
 wordArray[count] = input.nextLine();
 count++;
 }
 }catch(FileNotFoundException e) {
 e.printStackTrace();
 }
 return wordArray;
 }
 /**
 *Writes to the file 'words.txt' 
 *
 *@param word -Word from UserInput class
 */
 public void writeToFile(String word) {
 try {
 FileWriter writer = new FileWriter(file, true); 
 try {
 performWriteToFile(writer, word); // calls a method to write to file to make it cleaner
 }finally {
 writer.close();
 }
 }catch(IOException e) {
 }
 }
 // Writes word to file
 public void performWriteToFile(FileWriter writer, String word) throws IOException {
 writer.write(word + "\n");
 }
 // Checks the length of the file, uses that information to determine array size of the word array
 public int fileLength() {
 Scanner fileRead;
 int lines = 0;
 try{
 fileRead = new Scanner(file);
 //Iterates through the lines while there is a another one
 while(fileRead.hasNextLine()) { 
 lines++;
 fileRead.nextLine();
 }
 }catch(FileNotFoundException e) {
 e.printStackTrace();
 }
 return lines; // returns the number of lines
 }
}

Game Class

public class Game {
 private boolean running = true;
 private String hangmanWord;
 private char[] hangmanWordDisplay;
 private Word word = new Word();
 private char[] usedCharacters = new char[27];
 private String[] usedWords = new String[7];
 private String[] allWords;
 private int misses = 0;
 private boolean win = false;
 /**
 *Game class constructor that gets the hangman word, calls the method to read the 'word.txt file' 
 *and sets hangmanWordDisplay variable
 */
 public Game(){
 FileHandling filehandling = new FileHandling();
 allWords = filehandling.readFile();
 hangmanWord = word.selectWord(allWords);
 hangmanWordDisplay = new char[hangmanWord.length()];
 for(int i = 0; i < hangmanWord.length(); i++) {
 hangmanWordDisplay[i] = '_'; 
 }
 }
 // Method containing the game loop
 public void gameLoop() {
 Board board = new Board();
 UserInput userinput = new UserInput();
 // Game loop
 while(running) {
 board.printBoard(hangmanWordDisplay, usedCharacters, usedWords);
 String character = userinput.gameInput();
 inputChecking(character);
 }
 if(win){
 board.printBoard(hangmanWordDisplay, usedCharacters, usedWords);
 System.out.println("You have won");
 }else {
 System.out.println("You have lost, the word you were looking for is " + hangmanWord);
 }
 }
 // Checks whether the input is a single character or if it is a word
 public void inputChecking(String characterOrWord) {
 if(characterOrWord.length() == 1) {
 if(Character.isLetter(characterOrWord.charAt(0))) {
 inputCheckingCharacter(characterOrWord.charAt(0));
 }else {
 System.out.println("What you entered is not a character");
 return;
 }
 }else {
 inputCheckingWord(characterOrWord);
 }
 }
 //Checks if the word has been used before and checks if the word is the correct word
 public void inputCheckingWord(String wordInput) {
 if(word.hasUsedWord(usedWords, wordInput)){
 System.out.println("You have already used this tried this word");
 }else {
 addToUsedWords(wordInput);
 if(word.testWord(wordInput, hangmanWord)) {
 running = false;
 }else {
 misses++;
 Board board = new Board();
 board.writeToBoard(misses);
 running = wrongInputGameOver();
 return;
 }
 }
 }
 //Checks if the character has been used before and checks if the character is correct 
 public void inputCheckingCharacter(char inputChar) {
 boolean correct = false;
 if(word.hasUsedChar(usedCharacters, inputChar)){
 System.out.println("You have already used this character");
 }else {
 addToUsedCharacters(inputChar);
 for(int i = 0; i < hangmanWord.length(); i++) {
 if(word.testCharacter(inputChar, hangmanWord.charAt(i))) {
 hangmanWordDisplay[i] = inputChar;
 correct = true;
 if(checkCharacterInputWin()){
 running = false;
 win = true;
 }
 }else {
 if(!correct && i == hangmanWord.length() - 1) {
 misses++;
 Board board = new Board();
 board.writeToBoard(misses);
 running = wrongInputGameOver();
 return;
 }
 }
 }
 }
 }
 public boolean checkCharacterInputWin() {
 boolean test = false;
 for(int i = 0; i < hangmanWordDisplay.length; i++) {
 if(word.equalsIgnoreCase(hangmanWordDisplay[i], hangmanWord.charAt(i))) {
 test = true;
 }else {
 return false;
 }
 }
 return true;
 }
 // Checks if the player loses because he has made to many mistakes
 public boolean wrongInputGameOver() {
 if(misses == 8) {
 return false;
 }
 return true;
 }
 //Adds the inputed words to the usedWords array
 public void addToUsedWords(String word) {
 for(int i = 0; i < usedWords.length; i++) {
 if(usedWords[i] == null) {
 usedWords[i] = word;
 break;
 }
 }
 }
 //Adds the char to the usedCharacter array
 public void addToUsedCharacters(char inputChar) {
 for(int i = 0; i < usedCharacters.length; i++) {
 if(usedCharacters[i] == '\u0000') {
 usedCharacters[i] = inputChar;
 break;
 }
 }
 }
}

• maaartinus: Could you make it available somewhere for download as a single file? The CR site is too bad for doing a CR.
• Piwoot: speedyshare.com/aFDvb/Hangman.rar Is this ok?
• tim: for follow-ups, it's always good to link to the original question. Also, you should update your code so that it compiles properly (remove the 1 in Menu and rename word parameter in Word:hasUsedWord()). But just from a quick once-over, this looks like a major improvement to your original code. Congratulations.
• maaartinus: Sure, it's fine for me (I'd personally zip it as it's a more common format and place it to my Dropbox public folder for a direct download link, but that doesn't really matter).
• Piwoot: I did remove the 1 from menu, but when the moderator edited the post again he brought it back. Anyways fixed it again now, and thank you :)

Answer 1

Exiting

The code is pretty long and I've just complained on meta that there's no way to view it properly. So I restrict myself to a few comments and let others to comment on other aspects.

default:
 System.exit(0);

I'd never do that. It's like placing a bomb in the code. One day someone will want to reuse a part of it and there's no easy way to find out why it terminates.

Return a boolean and let the caller handle it. Throw a HangmanShouldStopException if using a boolean would be too clumsy. Do whatever but System.exit.

I also don't see why entering an illegal should stop the program instead of asking for a fix.

How to it:

 while (true) {
 System.out.println();
 Menu menu = new Menu();
 if (menu.menuChoice(menu.menuSelect())) break;
 }
/** ...
 @return true if the program should terminate
*/
public void menuChoice(int option) {...}

Main loop

You're creating a new Menu instance in each iteration. This is fine, but has no good reason. I'd go for something like this instead

• Create an instance before the loop
• Make filehandling, etc. to private members. Maybe not all of them, if it can't be done easily, then maybe it shouldn't be done. Just give it a try.

Change the handling for default to simply printing "Hey, what the heck you mean by " + option and doing nothing else. This way, the user has to enter a 4 for exiting.

Exceptions

} catch (IOException e) {
 e.printStackTrace();
}

This is the third worst possible error handling (the first is silent swallowing, the second is printing e.getMessage()). The question is what else can you do... I'd suggest to do nothing. Declare the FileHandling constructor with throws IOException and you're done here.

But somewhere it must be handled, mustn't it? Usually yes, with accent on somewhere. When the exception bubbles up, you may be able to do something with it. You wanted to write a file for the user and it failed? So tell the user (and add the exception trace for the developer). You failed to read a file without which nothing works? So tell the user and let the program crash. The important thing is handling exceptions as close to the top as possible, where you have more context.

Throwing constructor

A throwing constructor is a tougher problem than a method, as when it throws, you get no object to work with. If a variable or a field gets initialized like in

public void menuChoice(int option) {
 FileHandling filehandling = new FileHandling();
 ...
}

and the constructor throws, then you can't access the variable anywhere. The arising questions are many:

• Can the program do anything when word.txt can't be accessed.
  • If no, then let it die.
  • Otherwise, move the "dangerous" operation out of the constructor into some init method.
• Do other methods need the file word.txt?
  • If no, then consider making them static.
  • Otherwise, find out if there's anything you can do without it.

To me, it looks like without the file, nothing works. So just declare all methods using it with throws IOException. Add a catch clause to main, print something like "Out of luck today, word.txt can't be accessed.", print the stack trace, and rethrow. Or call System.exit(1), it's OK to do it in the main method as it's not meant to be reused.

Conventions

if(Word.equalsIgnoreCase(word)) {

No... variables always start with a lowercase letter. No need to make any exceptions, having both Word and word is just confusing.

There's a single space after if or while to distinguish it from method calls.

}catch(InputMismatchException e) {

and also before and after catch.

• Piwoot: Could you give an example that applies to the code? I don't quite understand how you want me to do it.
• Piwoot: Could you use the constructor in my FileHandling class and show me how you would go about doing it? I am a little uncertain about how the end result should look. Referrering to exception segment, and anyways thanks for your time.

Answer 2

As I said in the comments, your code improved a lot, so that's really good.

You should try out code analysis tools such as FindBugs, PMD, or Checkstyle. You don't have to listen to everything they say, but it's a good way to improve your code.

They would for example warn you:

• to not use exit() (use an exception or setting a boolean to false instead)
• that fileRead = new Scanner(file); in FileHandling:FileHandling() has no purpose
• about too long variable names (hangmanWordDisplay, which could be wordDisplay or character)
• about unused variables (test in Game:checkCharacterInputWin())
• about general formating (inconsistent use of curly brackets, line length, spaces, etc)
• about magic numbers
• about missing public/private modifiers
• not to catch and ignore exceptions. At least print them, otherwise it will be very hard to track down errors
• and a lot more

Anyways, here are some suggestions I would have:

• use less System.out.print. It's long and slow, and if you want to change it at some point, the more you use it, the more places to change. I would accumulate the output in a String or StringBuilder and then print it at the end (or create an output interface and pass it on to that).
• you don't have to iterate over a char array to print it, you can just pass the array directly to print. If you don't want to do that, create your own charArrayToString method to avoid duplicate code. Or even your own generic array to string method (the signature might look like this: <T> String arrayToString(T[] array, String appendToEach)).
• if (check) { return true; } return false; can be written as return check;.
• variable scoping: define your variables in the smallest scope possible. wordArray for example doesn't need to be a field, just define it directly in FileHandling:readFile().
• use early returns to reduce if-nesting. For example in Game:inputCheckingCharacter(), after checking if the character has been used, return. Then, you can remove the else statement (not the content of it though :) ).

This:

 for(String word : usedWords) {
 if(word == null) { //avoids null pointer exception by telling ignore the rest of the code if the array element is empty
 continue;
 }
 if(word.equalsIgnoreCase(word1)) {
 return true;
 }
 }
 return false;

is a bit confusing. I would rewrite it like this:

public boolean hasUsedWord(String[] usedWords, String word1) {
 for(String word : usedWords) {
 if(word != null && word.equalsIgnoreCase(word1)) {
 return true;
 }
 }
 return false;
}

Answer 3

Let's start with an alternative, more object oriented design. Then I'll make some comments about things you can improve in your code.

You should try to clearly separate your game logic with what you need to do for handling input/output. Consider for instance the following class sketch to represent a game.

public class Game
{
 public Game(string word)
 {
 // Store the word and initialise the game state
 }
 public Status processCharacter(char c)
 {
 /*
 * I assume you have a status enum
 * representing Victory, StillPlaying, GameOver.
 * 
 * You should process one character at
 * a time, update game state and return
 * what is the status of the game.
 * 
 * You should NOT do any input/output
 * in this class
 */
 }
 /*
 * Add all the methods you need to observe the state of the game
 * They should NOT mutate the state, just return information
 * about it.
 */
}

Once you encapsulate all the logic in that class, you can think at another class to encapsulate all the user interaction. That class should implement the following interface.

public interface UserInterface
{
 Action getNextAction();
 char getNextCharacter();
 void showGameState(Game game);
 void showGameResult(Status gameStatus);
}

That class should contain a reference to an InputStream and to an OutputStream and it should completely handle input validation and the rendering of the game state on the screen. Action should represent an action of you game such as create a new game, exit, ...

I don't think it is responsibility of your application to maintain a list of words in an external file. You can use a class to read from the file, but you should not write to it. You should just have a WordProvider implementing a string getRandomWord() method.

Finally you need to put everything together. What about this simple game loop in the main method. Note that you should really decompose it in some sub-methods but I think that for now it would be ok if you manage to get to an implementation that works in this way.

public static void main(String args[])
{
 // Initialise everything
 UserInterface userInterface = new UserInterface(/* pass a reference to standard input and output */);
 WordProvider wordProvider = new WordProvider(/* ... */);
 boolean shouldExit = false;
 do
 {
 switch(userInterface.getNextAction())
 {
 case Play:
 Game game = new Game(wordProvider.getRandomWord());
 Status status;
 while((status = game.processCharacter(userInterface.getNextCharacter()) == StillPlaying)
 {
 userInterface.ShowGameState(game);
 }
 userInterface.ShowGameResult(status); 
 break;
 case Exit:
 shouldExit = true;
 break;
 }
 }while(!shouldExit);
}

Let's see what I don't like in your code.

Object initialisation

The first thing I noticed is that you create new objects all the time when you probably don't really need to. Why do you need to create a new Menu instance in main? Why do you need to create a new UserInput instance in menuSelect and in menuChoice? They all look classes that you should instantiate only once in your application. I think you need to review what an object is and how you should design object oriented programs.

Misplaced responsibilities

Why do you have this statement menu.menuChoice(menu.menuSelect());? menu should not both handle choice of what to do next and getting its input. Input should come from an external source.

Naming

How does your Word class represent a word? It doesn't look it does to me. Try to use names that really represent what your class does. If you cannot thing at a meaningful name you probably want to revise your design until everything has a natural and clear name (and hence has a well defined role).

Answer 4

//Math function returning a random number
public int randWord(int length) {
 return (int)(Math.random() * length);
}

A better way to return a random number is to use the Random class. random.nextInt(length); is preferred. Also, this method does not need to be public, it should be private as it is only called from this method:

public String selectWord(String[] words) {
 return words[randWord(words.length)];
}

I'd suggest that you take a look at the randomElement method in my answer here and use that method. Note that when using a Random object, you should only create it once. So you can put this inside your class:

private final Random random = new Random();

---

You have three methods that all three have unnecessary number of lines, and can all be static. And all should use proper Javadoc instead of a simple comment above the method.

/**
 * Returns true if the character entered by the user is equal to the character sent from the word in the game class
 */
public boolean testCharacter(char testChar, char charFromWord) {
 return equalsIgnoreCase(testChar, charFromWord);
}
/**
 * Method that uses .toLowerCase to make sure the if test ignores the case of the characters
 */
public boolean equalsIgnoreCase(char testChar, char charFromWord) {
 return Character.toLowerCase(testChar) == Character.toLowerCase(charFromWord);
}
/**
 * Checks if the word the user entered is equal to the word selected as the hangman word
 */
public boolean testWord(String testWord, String hangmanWord) {
 return testWord.equalsIgnoreCase(hangmanWord);
}