Hangman game in Java - second try

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 = {
{'_', '_', '_', '_', '_', '_', '_', '_', '_', '_',

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.

StackExchange Code Review Q#64427, answer score: 12