patternjavaMinor
Very simple Hangman game
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
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
Redundancy: The core logic between
One thing to point out also is that in the current implementation,
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.