4
\$\begingroup\$

This simple implementation of the HangMan game in Java is a side project, unlike the usual programs I write which are either school assignments or problems on coding websites.

It feels great to have something simple working, but I know for sure this can be improved. This only runs through the command line now, but I'm thinking of adding a lot more to this in the near future.

import java.io.File;
import java.io.FileNotFoundException;
import java.util.Scanner;

public class Main {


    public static void main(String[] args) throws FileNotFoundException {

        Scanner userGuess = new Scanner(System.in);

        printTitleScreen();
        System.out.println("");
        String word = getRandomWordFromFile("/Users/kingcold/Desktop/words.txt");
        char[] wordCharArray = word.toCharArray();

        System.out.println("It's time to guess the word!");

        StringBuilder gameWord = new StringBuilder(printBlankSpaces(word));

        int successfulAttempts = 0;
        int failedAttempts = 0;
        int totalAttempts = 0;


        do {

            System.out.println("Your word looks like this: " + gameWord);
            System.out.println("Your guess: ");

            String userInput = userGuess.next();
            char charUserInput = userInput.charAt(0);

            if (isValidInput(userInput)) {
                    System.out.println("Your guess is: " + userInput);

                    if (!(containsUserInput(wordCharArray, userInput))) {

                        letterNotInWordMessage(userInput);
                        failedAttempts++;

                    } else if (containsUserInput(wordCharArray, userInput)){

                        letterInWordMessage(userInput);     
                        gameWord.setCharAt(getMatchingIndex(wordCharArray, userInput), charUserInput);
                        successfulAttempts++;

                    }

            } else {

                System.out.println("Invalid input entered. Try again.");

            }

            totalAttempts = successfulAttempts + failedAttempts;


        } while (!word.equals(gameWord.toString()));


        if (word.equals(gameWord.toString())) {
            System.out.println("Congratulations, you correctly guessed the word!");
            System.out.println("The word is " + word);
            System.out.println("You guessed the word in " + totalAttempts + " attempts");
            userGuess.close();
        }

    }

    private static void printTitleScreen() {
        System.out.println("-------------------");
        System.out.println("Welcome to Hangman!");
        System.out.println("-------------------");
    }

    private static String printBlankSpaces(String word) {
        return new String(new char[word.length()]).replace('\0', '-');
    }

    private static void letterNotInWordMessage(String letter) {
        System.out.println("The letter " + letter + " is not in the word!");
    }

    private static void letterInWordMessage(String letter) {
        System.out.println("The letter " + letter + " is in the word!");
    }

    private static int getMatchingIndex(char[] wordArray, String userGuess) {
        int index = -1;
        for (int i = 0; i < wordArray.length && (index == -1); i++) {
            if (String.valueOf(wordArray[i]).equals(userGuess)) {
                index = i;
            }
        }
        return index;
    }

    private static boolean containsUserInput(char[] wordArray, String userGuess) {
        for (int i = 0; i < wordArray.length; i++) {
            if (wordArray[i] == userGuess.charAt(0)) {
                return true;
            }
        }
        return false;
    }

    private static boolean isValidInput(String userGuess) {
        final int VALID_INPUT_LENGTH = 1;
        if (userGuess.length() == VALID_INPUT_LENGTH  && Character.isLetter(userGuess.charAt(0))) {
            return true;
        } 
        return false;
    }

    private static String getRandomWordFromFile(String fileName) throws FileNotFoundException {

        File file = new File(fileName);
        Scanner fileScanner = new Scanner(file);
        String fileContents = "";

        while(fileScanner.hasNextLine()) {
            fileContents = fileContents.concat(fileScanner.nextLine() + "\n");
        }

        fileScanner.close();
        String[] fileContentArray = fileContents.split("\n");
        return fileContentArray[(int) (Math.random() * fileContentArray.length)];

    }


}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Let’s start with your utility methods.


printBlankSpaces(String word)

This method returns a new String; It does not do any printing. It needs a better name...


getMatchingIndex(char[] wordArray, String userInput)

You repeatedly turn each character of wordArray[] into a String using String.valueOf(...). It would be better to turn the userInput into a character before the loop starts (using .charAt(0)), and just do a simple comparison in the loop with that character.

It would be even better to pass in a char userInput, instead of a String.

Instead of && (index == -1) in your loop condition, just break; out of the loop after you do the assignment index = i;, or even simplier, return i; and you could remove the index variable entirely.


containsUserInput(char[] wordArray, String userInput)

Instead of extracting .charAt(0) in every iteration of the loop, do it before the start of the loop and save it in a local variable. Or even better, like above, pass in char userInput.

You aren’t using the loop index other than for retrieving the character wordArray[i]. This is a sign you could use an enhanced for-loop:

char userChar = userInput.charAt(0);
for (char letter : wordArray) {
   if (letter == userChar) {
       return true;
   }
}
return false;

Or even simpler, the entire method could be one statement, leveraging the function above:

return getMatchingIndex(wordArray, userInput) >= 0;

isValidInput(String userGuess)

The construct if (condition) return true; else return false; can be re-written as return condition;. You don’t need the if statement.


getRandomWordFromFile(String fileName)

You are doing a lot of work to get a random line from the file. First, you are reading the file, line-by-line. Then, you concatenate all the lines together! Finally you split the concatenation back into the individual lines!

If you simply stored each line as you read it into a List<String>, you would be able to skip the concatenation and wouldn’t need to split later.

In fact, reading all the lines of a file into a List<String> is such a common operation, there is a standard method for it. List<String> Files.readAllLines(Path path).

With that, the function could be reduced to two lines:

List<String> lines = Files.readAllLines(Paths.get(fileName));
return lines[ ThreadLocalRandom.current().nextInt(lines.size()) ];

That skips over a lot of fine points. Most important is ensuring all file resources are properly closed, even in the face of exceptions. The “try-with-resources” statement is best used for that:

File file = new File(fileName);
List<String> lines = new ArrayList<>();
try(Scanner scanner = new Scanner(file)) {
    while(scanner.hasNextLine()) {
        lines.add( scanner.nextLine());
    }
}

All that is done for you with the one readAllLines() call.


main

charUserInput is not used. As mentioned above, many of your methods would be easier to write if they were given a single character, instead of a String ... and you have this unused character variable that would be perfect for that...

You tests if (!(containsUserInput(wordArray, userInput))) { ... } followed immediately by else if (containsUserInput(wordArray, userInput)) { ... }. Those tests conditions are - literally - opposites of each other; if one is true, the other must be false, and vis-versa. Instead of repeating the test, both in code and in CPU time, just use an else

if (!containsUserInput(wordArray, userInput)) {
   ...
} else {
   ...
}

I’ve removed the unnecessary ( )’s. You can improve this further by switching the order of the then-else clauses, and removing the ! operator.

if (containsUserInput(wordArray, userInput)) {
    ... guess is correct code ...
} else {
    ... guess is incorrect code ...
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Such a good answer, except for the last sentence suggesting snake_case for local variables. The naming conventions say these variables are also written in camelCase. \$\endgroup\$ Mar 17, 2019 at 5:30
  • \$\begingroup\$ Upvoted for all the good advice, but I want to explicitly state that I have the same gripe with this answer as Roland does... \$\endgroup\$
    – Vogel612
    Mar 17, 2019 at 16:48
  • \$\begingroup\$ Ok ... ok ... there is a badge for caving in to peer pressure, right? I’ll remove last sentence. \$\endgroup\$
    – AJNeufeld
    Mar 18, 2019 at 1:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.