Return to Answer

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.

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.

##Conventions

###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

##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.

##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.

##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.