HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Very simple Hangman game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gamesimplehangmanvery

Problem

Does this code follow standard conventions, not redundant? How I can make it more comprehensible for other people?

It's an exercise from the Java introductory book:


Write a hangman game that randomly generates a word and
prompts the user to guess one letter at a time. Each letter in the word is displayed as an asterisk. When the user makes a correct guess, the actual letter is then displayed. When the user finishes a word, display the number of misses.

My program:

```
import java.util.Arrays;
import java.util.Scanner;

public class Hangman{
public static void main(String[] args) {
String[] words = {"writer", "that", "program"};
// Pick random index of words array
int randomWordNumber = (int) (Math.random() * words.length);
// Create an array to store already entered letters
char[] enteredLetters = new char[words[randomWordNumber].length()];
int triesCount = 0;
boolean wordIsGuessed = false;
do {
// infinitely iterate through cycle as long as enterLetter returns true
// if enterLetter returns false that means user guessed all the letters
// in the word e. g. no asterisks were printed by printWord
switch (enterLetter(words[randomWordNumber], enteredLetters)) {
case 0:
triesCount++;
break;
case 1:
triesCount++;
break;
case 2:
break;
case 3:
wordIsGuessed = true;
break;
}
} while (! wordIsGuessed);
System.out.println("\nThe word is " + words[randomWordNumber] +
" You missed " + (triesCount -findEmptyPosition(enteredLetters)) +
" time(s)");
}

/* Hint user to enter a guess letter,
returns 0 if letter entered is not in the word (counts as try),
returns 1 if letter were entered 1st time (counts as try),
returns 2 if already guessed letter was

Solution

Magic Numbers: It would be better to move your definitions for 0,1,2,3 into an enum or final fields so that you can reference them with a meaningful name. For example final int LETTER_NOT_IN_WORD = 0 or enum HangmanGuess { LETTER_NOT_IN_WORD }

Redundancy: The core logic between findEmptyLetters and inEmptyLetters is the same, you are looking for a char in a char[]. You could refactor like this:

/* Check if letter is in enteredLetters array */
public static boolean inEnteredLetters(char letter, char[] enteredLetters) {
    return indexOf(letter, enteredLetters) >= 0;
}

/* Find first empty position in array of entered letters (one with code \u0000) */
public static int findEmptyPosition(char[] enteredLetters) {
    return indexOf('\u0000', enteredLetters)
}

/* Determine the index in {@code vals} where {@code ch} exists. Returns -1 if {@code ch} is not in {@code vals}. */
public static int indexOf(char ch, char[] vals) { 
    return Arrays.asList(vals).indexOf(Character.valueOf(ch));
}


One thing to point out also is that in the current implementation, findEmptyPosition will return n where n is the enteredLetters.length + 1 if there are no empty positions. I'm not sure if this is the desired functionality.

Code Snippets

/* Check if letter is in enteredLetters array */
public static boolean inEnteredLetters(char letter, char[] enteredLetters) {
    return indexOf(letter, enteredLetters) >= 0;
}

/* Find first empty position in array of entered letters (one with code \u0000) */
public static int findEmptyPosition(char[] enteredLetters) {
    return indexOf('\u0000', enteredLetters)
}

/* Determine the index in {@code vals} where {@code ch} exists. Returns -1 if {@code ch} is not in {@code vals}. */
public static int indexOf(char ch, char[] vals) { 
    return Arrays.asList(vals).indexOf(Character.valueOf(ch));
}

Context

StackExchange Code Review Q#95426, answer score: 5

Revisions (0)

No revisions yet.