Just quickly, here's some feedback:
I think your set up code is fine (where you're creating the JFrames etc), it's very boiler platey, but that's java.
But you shouldn't put that logic in the constructor. Put it in an
init()
method, called after construction.Do use scope declarations, ie. make that
Hangman()
constructorpublic Hangman()
. Same withvoid testLetter
is this method meant to be public or private? Make it explicit so that others (and yourself, in six months!) know how the code is meant to be used.Consider separating your Swing/GUI logic, and your game logic to different classes. ie, something like
HangmanApp
(your main class) contains aHangmanGui
and aHangmanLogic
.You have magic numbers all through your code. For example this line is particularly bad:
textContents.setText("Incorrect Guess! You have " + (6 - incorrectGuesses) + " left.");
What if you want to change the number of guesses? Ideally these configurations would be set in a properties file, but for a small application setting them at the top of the class ifis fine.Your
if else
branching intestLetter
is scary. As it currently is, I can't immediately see what each branch is for. Comments would help on each branch, but I would move 'test if the letter fits' logic, and 'what should I do if letter is correct/incorrect' logic, into their own private methods.
eg. something like:
void testLetter(String a) {
if (evaluateGuess(a)){ //returns true if guess is correct
handleCorrectGuess(a);
}
else (
handleIncorrectGuess(a);
}
}
- You don't need to use a
==
comparison on booleans. Use a descriptive variable name and!
operator instead. ie.if (!gameLost){...
orif(gameStillInProgress){...