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
manpreet
Best Answer
2 years ago
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: