Return to Answer

• 35.3k
• 5
• 41
• 103

• 35.3k
• 5
• 41
• 103

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

Naming conventions. Local variables, including method arguments, should not be written in mixedCase. Instead, use identifiers like word_array, user_input, file_name etc.