Jessica’s Code Review
Variable Naming
The variable randIndex could be more clearly named randomIndex. Adding the extra letters improves readability without increasing complexity.
Similarly, the Scanner variable scnr would be clearer as scanner. Using full, descriptive names helps make the code easier to follow.
Logic and Test Failures
Two unit tests are failing. The getHint test fails because the loop does not break after decrementing numberOfHints. I recommend using the debugger or adding temporary print statements to trace the loop’s behavior—specifically, printing the value of i at each iteration and tracking how many times numberOfHints decreases.
The second failing test is harder to diagnose. Since I’m less familiar with do-while loops and find them less intuitive, it might help to rewrite that logic using a standard for loop. Doing so could make the flow clearer and potentially resolve the issue.
Comments and Documentation
The code contains a large number of comments—almost one on every line. While Javadoc above each method is helpful and appropriate, many inline comments explain logic that is already self-explanatory (e.g., if (numberOfHints <= 0)). Removing unnecessary comments will make the code cleaner and easier to read.
Cris’ Code Review
Tests and Functionality
All tests pass, which is excellent.
Readability and Naming
Variable names are clear and descriptive, making the code easy to understand.
Comments and Documentation
The comments are generally helpful, but there are more than necessary in some places. For example, commenting on what break; does isn’t needed. Your code is already very readable and doesn’t require that level of explanation. Reducing unnecessary comments will make the file even cleaner.
Reviews I Received
My peers felt that my variable names were clear, meaningful, and made it easy to understand what each variable was used for. They didn’t find any logic that needed to be made more efficient; the code seemed concise, with no unnecessary branching or extra variables. There were no unused imports or errors, and the only minor warning came from IntelliJ making assumptions about my debug flag.
They said my formatting was clean and easy to read, with good spacing between code blocks. The only small note was that in my chooseWord() method, the lines weren’t separated the same way as in the rest of the file, but this was more of a nitpick than a real issue.
Regarding comments, they liked that all of my methods were well-documented. Their main suggestion was to include more inline comments within the methods themselves so it’s easier to understand each chunk of logic at a glance later on.
Answers to Questions
What improvements would you make to your code / what was suggested?
After reviewing my Hangman implementation, there are a few improvements I would make.
chooseWord():
My method loops through every word and picks the first unguessed one. It should instead stop after choosing one word or use an actual random selection. Also, the for loop currently doesn’t break, so it unnecessarily continues scanning. I need to break the loop once a word is chosen or rewrite the logic using a proper random index.Upper/lowercase consistency in makeGuess():
I converted letters to lowercase but my secret words are uppercase from the file. This works, but it’s inconsistent. I might standardize everything to one case.
Which unit tests were the hardest to pass?
The hardest tests to pass were:
hintTest():
This test came up with edge cases when hints should not trigger a win and whether the hint system respects the count exactly.testDisplayGameState():
Comparing exact output formatting forced me to match every newline, spacing, and list formatting exactly.
How do the existing tests function, and could they be improved?
They build a temporary file, load words, and simulate real Hangman gameplay by guessing letters until win/lose conditions are met. The tests also verify individual helper methods (reading files, picking words, scoring, hints, tracking guesses).
How they could be improved:
chooseWord(): Should test that the method only selects unguessed words and actually uses randomness. I did not use a Random object, but I still passed the tests.
testDisplayGameState(): Testing the returned string only would be cleaner.
checkPlay(): It could assert that the game eventually ends.
Do the existing unit tests cover the full range of the subclasses?
There aren’t any sub classes in my implementation.
How would you change the unit tests?
Add assertions in checkPlay().
Add a test for case-insensitive guessing (guessing lowercase words when the file includes uppercase words).
What did you struggle with?
I struggled with getting the hint system to behave exactly as the tests expected. Making sure guessedWord updated the correct characters without accidentally replacing others. Understanding how strict the comparison was for displayGameState.
What did one of your teammates struggle with?
One of the teammates struggled with getting getNumberOfHints tests to pass. I found a small bug in their code. They needed to break out of their loop after decrementing hints.
Was any part of writing the code easy for YOU?
Implementing the simple getters was very straightforward. hasWon(), hasLost(), and getScore() were easy once the main logic worked. readFile() was also easy since this was done in labs before.
What was your biggest HW1 victory?
My biggest victory was getting the entire suite of unit tests to pass without modifying the tests themselves. Some of the tests (especially hintTest and checkLose) revealed subtle bugs in my logic, and fixing those felt like a major win.
No comments:
Post a Comment