Java Hangman implementation

Question

I'm learning Java and plan to take the OCA Java SE8 exam pretty soon. I made a fairly primitive Hangman game to aid with the learning. I would love some feedback on it if possible. Anything that's not done to required standards (naming conventions, how quick some of the methods are etc), I would love to know.

The two things I'm not happy with is that:

1. I am not sure how to mask user input, at the moment player 2 can see the word to be guessed.
2. This game is done line by line. I'm not sure how to make it more 'static' i.e the asterisks are in one place and it doesn't move and there is a line of words already guessed (I hope that makes sense). I am trying to avoid using a GUI.

package hangman2;
import java.util.Arrays;
import java.util.Scanner;

public class Hangman2 {

static int guessesRemaining;
static String stringWord;
static char[] charWord;

public static void playHangman() {

    System.out.print("Welcome to Hangman! ");
    System.out.println("Player 1: Enter a word");
    Scanner scan = new Scanner(System.in);
    stringWord = scan.nextLine();

    //Loop to get user to enter in letters only
    while (stringWord.matches(".*[^a-z].*")) {
        System.out.println("Please enter letters only, try again:");
        stringWord = scan.nextLine();
    }

    System.out.println("The length of the word is " + stringWord.length()); //Print out length of word.
    stringWord = stringWord.toLowerCase();

    int lettersRemaining = stringWord.length(); //Letters to be gussed is equal to the word length.

    //Create a new char array equal to the word length, add * to each index.
    charWord = new char[stringWord.length()];
    for (int i = 0; i < stringWord.length(); i++) {
        charWord[i] = ('\u002A');
    }
    int livesRemaining = 7; // 7 lives remaining!
    MainLoop:
    /*
    Main loop, gets user input using Scanner, ensures it is only a letter (not a number or special character)
    Converts string to char, uses the guess method which compares the secret word to the character entered and
    returns true if it found it, false if it didn't. 
    If found, letters remaining - 1, if the char array contains any asterisks, envoke the gameWon() method.
    If not found, lives remaining - 1, if no more lives, envoke the gameLost() method.
    */
    while (livesRemaining > 0) {
        System.out.println("Player 2: Guess a Letter:!");
        char guessLetter;
        String b = scan.next();
        while (b.matches(".*[^a-z].*")) {
            System.out.println("Please enter letters only, try again:");
            b = scan.next();
        }
        guessLetter = b.toLowerCase().charAt(0);
        String a = new Hangman2().print(guessLetter);
        if (guess(guessLetter, stringWord)) {
            lettersRemaining--;
            if (!a.contains("*")) {
                gameWon();
                break;
            }
            System.out.println("Well done, you guessed correctly! You have " + lettersRemaining + " letters remaining");
        } else {
            livesRemaining--;
            if (livesRemaining == 0) {
                gameLost();
            }
            System.out.println("Too bad, you guessed incorrectly, you have " + livesRemaining + " lives remaining");
        }
    }
}

public String print(char c) {
    //Prints asterisks and replaces characters that have been guessed.
    String misses;
    char[] guess = stringWord.toCharArray();
    for (int i = 0; i < charWord.length; i++) {
        if (guess[i] == c) {
            charWord[i] = c;
        }
    }
    misses = Arrays.toString(charWord);
    System.out.print(misses + "  ");
    return misses;
}

public static boolean guess(char c, String s) {
    //Evaluates the chracter entered and compares it to the word, returns a boolean (correct or incorrect).
    boolean result = false;
    char[] a = s.toCharArray();
    for (int i = 0; i < s.length(); i++) {
        if (a[i] == c) {
            result = true;
        }
    }
    return result;
}

public static void gameWon() {
    char yesno;
    boolean enter;
    System.out.print("Congratulations, you won!");
    Scanner input = new Scanner(System.in);
    System.out.println(" Would you like to play again? Y / N");
    do {
        yesno = input.next().toUpperCase().charAt(0);
        switch (yesno) {
            case 'Y':
                playHangman();
            case 'N':
                System.exit(0);
            default:
                System.out.println("Please enter Y / N");
                enter = false;
                break;
        }
    } while (!enter);
}

public static void gameLost() {
    char yesno;
    boolean enter;
    System.out.print("Too bad, you lost...");
    Scanner input = new Scanner(System.in);
    System.out.println(" Would you like to play again? Y / N");
    do {
        yesno = input.next().toUpperCase().charAt(0);
        switch (yesno) {
            case 'Y':
                playHangman();
            case 'N':
                System.exit(0);
            default:
                System.out.println("Please enter Y / N");
                enter = false;
                break;
        }
    } while (!enter);
}

public static void main(String[] args) {
    playHangman();
}

}

Answer 1

Classes are not objects

static int guessesRemaining;
static String stringWord;
static char[] charWord;

You are making the entire class into a sort of fake object.

    private String stringWord;
    private char[] charWord;

That's how real object fields should look.

Unless you have some specific reason (e.g. you want a field to be visible outside for some specific reason), you should declare object fields as private visibility. Avoid the default (package private) unless you have some reason to use it. And not a "I might want to ... in the future" type reason. A reason that is actually implemented in your code. In this case, your entire program is one class. Any visibility will work, but private is preferred in most cases.

Again, I'm not saying that there is never a reason to use visibilities other than private for object fields (the data stored in an object). I'm saying that you don't have such a reason here. I'm also saying that it is probably easier to always make fields private and fix the rare exceptions later than to try looking for reasons to make it anything else in the beginning. If that policy starts to chafe, rethink it. But in the early going, it will rarely chafe beyond setters and getters.

The reason why you don't want to use a class as a fake object is that it precludes you from making multiple instances of the class. You can't play two Hangman games at once this way.

You'll need a constructor.

    Hangman2(String word) {
        stringWord = word;
        charWord = new char[word.length()];
        Arrays.fill(charWord, '*');
    }

This also eliminates the need to manually fill the character array with asterisks. Instead we use the built-in Arrays.fill.

And while some methods do not need to be static, main does. So rewrite it something like

    public static void main(String[] args) {
        do {
            Hangman2 game = new Hangman2(inputWordToGuess());
            game.play();
        } while (isReplayDesired());
    }

Note that both getWordToGuess and isReplayDesired would be static methods. Meanwhile, play isn't.

When to use static

When it does make sense to use static is when you are using the same thing everywhere. For example, it would make sense to have a static Scanner.

    public static Scanner input = new Scanner(System.in);

This can be public as well, so other classes don't have to make their own scanners.

In your original, you would build a scanner object in any method that happened to need to fetch user input. This way, you can build one that lasts for the entire program.

Don't use redundant code

Your gameWon and gameLost methods are almost identical. So I replaced both with

    public static boolean isReplayDesired() {
        while (true) {
            System.out.println(" Would you like to play again? Y / N");
            switch (input.next().toUpperCase().charAt(0)) {
                case 'Y':
                    return true;
                case 'N':
                    return false;
                default:
                    System.out.println("Please enter Y / N");
            }
        }
    }

This gets rid of your enter Boolean. So less code.

You can use the intermediate yesno variable, but it's not necessary.

By returning a Boolean, we can simplify the code both here and in the caller.

It would be possible to overrun the stack with recursive calls with your original code (albeit somewhat difficult). Now it isn't. No recursive call.

I never put a break in the default clause when it's the last in the switch statement. It's not necessary. And I'll always put it last when I can, which is almost all the time. I do put it in the last non-default clause though, although it's still not necessary.

Try to keep methods single purpose

Your original playHangman method tries to do several things. I broke out getting the word. Now we have two simpler methods. This makes things more flexible. Instead of getting words from user input, I can switch to reading them from file without changing the play method at all. I just stop using inputWordToGuess.

If I change the name, I could reuse inputWordToGuess for something else.

Answer 2

public class Hangman2 {

HangMan2 is a bad name. Don't add numerals as class names. Add a final modifier to the class if you don't intend to override it.

static int guessesRemaining;
static String stringWord;
static char[] charWord;

You can write the code in purely OO manner, thus you don't need statics. Additionally, you may add a private modifier.

public static void playHangman() {

No need for static.

//Loop to get user to enter in letters only
while (stringWord.matches(".*[^a-z].*")) {
    System.out.println("Please enter letters only, try again:");
    stringWord = scan.nextLine();
}

This can be a different method. Regex can be declared as a string constant. You may not want infinite trial attempts.

stringWord = stringWord.toLowerCase();

Why not call it only word?

while (livesRemaining > 0) {

Clearly warrants a method of its own.

while (b.matches(".*[^a-z].*")) {

Comes back to my point of a constant. Have you made sure it does not lead to NPE if string is null?

String a = new Hangman2().print(guessLetter);

A print method is expected to be void, else change the name. a is a bad name.

if (!a.contains("*")) {

Can't this be replaced with letterRemaining == 0?

System.out.print(misses + "  ");

Assume you could print a string at a time?

char[] a = s.toCharArray();

You could opt for String.charAt(i).