git checkout -b hangman

Wk03 Learning Journal: HW 1 Code Review

I reviewed Josh Goldberg and Christopher Buenrostro’s hangman code.

For Josh’s code, this was my review:

1. Some things I noticed, though, I don't know if any of these things actually improve efficiency or memory; there is a constant variable PLACEHOLDER = "_", that could have been used on lines 121 and 272, instead of using the char '_' itself.

2. There's an unused import java.util.Random. I'm not sure if this was required to be used per the assignment or just something that didn't end up being needed and wasn't deleted.

3. One thing I did notice in the chooseWord() method, was that the do while loop checks for previously guessedWords, but guessed words are never added to guessedWords anywhere in the code ex: guessedWords.add(selectedWord);. 

For Christopher’s code, this was my review:

1. In the chooseWord() method, there is no guard or check before the while loop: while(guessedWords.contains(tempWord)), that checks to see if the guessedWords.size() == allWords.size(), and if so, clears guessedWords, ex: guessedWords.clear();. If all the words in the hangmanWord.txt file are used, that while loop would never exit.

2. Another small thing I noticed in the chooseWord method was line 129 used system.err.println, instead of system.out.println. Everywhere else you used system.out and the system.err is used for error messages.

3. Your Scanner in readFile() isn’t inside a try-with-resources block, so it never closes. That means the file handle stays open until Java’s garbage collector eventually frees it, which can lead to resource leaks or locked files. You can fix it easily by using try (Scanner sc = new Scanner(new File(fileToLoad))) { … }. That automatically closes the scanner as soon as you leave the block.

4. In the getScore() method, it is supposed to return the remainingGuess + score, per the Javadoc comments. Your method only returns the score.

Summary of Feedback on my code:

Only that I should update my hasWon method with:
return guessedWord.indexOf(PLACEHOLDER) == -1;
Instead of using:
return !guessedWord.toString().contains(PLACEHOLDER);

Identified Trends:

I noticed that both Josh and Christopher had updates that should be made in the chooseWord method. I identified this as a trend, as the chooseWord method gave me the most trouble to implement. Based on this, I think, many others may have had trouble in this method as well. Though the additions that both Chris and Josh should implement were different, it makes me curious to know how others wrote their code for that method.

Answers to Questions:

• Josh made an excellent suggestion in the hasWon method. I was originally returning:
  return !guessedWord.toString().contains(PLACEHOLDER);

Josh advised me that each time that statement ran, it created a String object, and thus memory was allocated for it. Instead, he recommended that I use the indexOf method: return guessedWord.indexOf(PLACEHOLDER) == -1;

This works directly with the StringBuilder; avoiding the conversion to a string. I implemented this into my code as I noticed that both Josh and Christopher used the indexOf method instead of creating a new String object, as was the case with my original code.

• The only unit test that was finicky was the testDisplayGameState, though, I am not sure why. The javadoc for this method seems to imply that the test might not work. I was only able to get it to pass when my entire program was finished.

• The existing tests verify that the Hangman class behaves correctly by checking object creation, file reading, word selection, game state display, win/loss conditions, hint use, and overall game flow. They confirm expected outputs through assertions like assertTrue, assertFalse, and assertEquals, using sample words to ensure each core method works as intended. They work but could be improved by removing randomness, adding edge-case checks (like repeat guesses and word reuse), and making more precise, deterministic assertions.

• I struggled most with the initialization order in the chooseWord method. The game state depends on a specific sequence: assign secretWord first, then derive remainingGuesses, compute numberOfHints from that, reset guessedLetters, and finally build the guessedWord mask using the secretWord length. Until I set those in the right order, the game appeared finished or skipped options. Getting this dependency chain right was the turning point.

• Josh mentioned “I had a huge mistake when I missed part of the exit spec and didn't set remaining guesses correctly. The whole test suite bombed.” Other than this, as mentioned above, both Chris and Josh have additions/updates to make in the chooseWord method, the method that gave me the most trouble to implement.

• I would say that most of the code was fairly easy to implement and I did not have too much trouble working through this assignment.

• My biggest victory for this assignment was getting it done efficiently and early in the week to move on to other assignments.