Skip to main content
edited body
Source Link
dwjohnston
  • 1.3k
  • 9
  • 18

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 with void 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 a HangmanGui and a HangmanLogic.

  • 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 in testLetter 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){... or if(gameStillInProgress){...

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 with void 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 a HangmanGui and a HangmanLogic.

  • 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 if fine.

  • Your if else branching in testLetter 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){... or if(gameStillInProgress){...

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 with void 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 a HangmanGui and a HangmanLogic.

  • 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 is fine.

  • Your if else branching in testLetter 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){... or if(gameStillInProgress){...
added 6 characters in body
Source Link
dwjohnston
  • 1.3k
  • 9
  • 18

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 with void 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 a HangmanGui and a HangmanLogic.

  • 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 if fine.

  • Your if else branching in testLetter 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 booleanvariable name and ! operator instead. ie. if (!gameLost){... or if(gameStillInProgress){...

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 with void 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 a HangmanGui and a HangmanLogic.

  • 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 if fine.

  • Your if else branching in testLetter 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 boolean and ! operator instead. ie. if (!gameLost){... or if(gameStillInProgress){...

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 with void 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 a HangmanGui and a HangmanLogic.

  • 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 if fine.

  • Your if else branching in testLetter 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){... or if(gameStillInProgress){...
added 686 characters in body
Source Link
dwjohnston
  • 1.3k
  • 9
  • 18

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 with void 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 a HangmanGui and a HangmanLogic.

  • 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 if fine.

  • Your if else branching in testLetter 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 boolean and ! operator instead. ie. if (!gameLost){... or if(gameStillInProgress){...

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 with void testLetter is this method meant to be public or private. Make it explicit so that others 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 a HangmanGui and a HangmanLogic.

  • 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 if fine.

  • Your if else branching in testLetter is scary. I would:

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 with void 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 a HangmanGui and a HangmanLogic.

  • 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 if fine.

  • Your if else branching in testLetter 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 boolean and ! operator instead. ie. if (!gameLost){... or if(gameStillInProgress){...
Source Link
dwjohnston
  • 1.3k
  • 9
  • 18
Loading