- Methods in the player class..can they live somewhere else, possibly game?
- Suggestions on populating the current puzzle onto the game board.
- Does the domUpdates file and the index.js file look like they are seperated appropriately?
- Suggestions on the guessLetter function.
- Feedback on the UI instructions for actually playing the game.
- Feedback on tests.
- Get together with your group and send a link of your team's GitHub Repo to the pair that is reviewing your project.
- Look over this guide with your project partner and highlight the pieces where your team can use assistance/you would like your reviewers to take a closer look. Fork this gist and send the forked copy over to the pair reviewing your code.
- Follow the instructions to clone down the repo you are given and start reviewing the codebase. Take notes for written feedback.
- Come back together in your larger group. This time should be used to have an in-person discussion about the feedback. Send your formal written feedback as a gist/document to the pair and your instructors.
While reviewing the code, ask yourself the following questions:
CODEBASE
- Does the code work? Does it perform its intended function, the logic is correct etc.
- Is all the code easily understood?
- Does it conform to Turing's Style Guide? These will usually cover location of braces, variable and function names, line length, indentations, formatting, and comments.
- Is there any redundant or duplicate code?
- Does the code follow the principle of single responsibility?
- Can any global variables be replaced?
- Is there any commented out code? Any console.logs?
- Do the names used in the application convey intent?
- Do tests exist, and are they comprehensive?
- Do unit tests actually test that the code is performing the intended functionality?
- Does the repo have a .gitignore file? Is there a .DS_Store file or node_modules directory committed that shouldn’t be?
README
- Does the README follow the conventions we discussed in class (i.e. Abstract listed first, install instructions second, everything else listed after)
- Is there a link to the DTR for the team available?
- Is there a wireframe for the project available?
- Is there anything that is confusing or hard to understand with what is written?
- Does the README do a goob of showcasing this project? If you were a random human who happened to come across this project, would you explore it further based on the current state of the README?
- What suggestions would you make for making the README better?
GITHUB/ISSUES/WORKFLOW
- Are issues being used to keep track of tasks, enhancements and bugs?
- Do the titles/descriptions of the issues provide all the relevant/necessary details for the issue? Would you be able to jump right in and assist?
- Are other strategies being used to keep track of issues (labels, milestones, assignments)?
- Is there effective communication happening in PRs outside of working school hours?
- Do the PRS follow the flow that we discussed in class?
- Summarize the changes that you made.
- Give the reason WHY you made those changes
- Ask for any insights
You will be writing a formal piece of feedback (with your partner) to give to the team whose project you reviewed this morning. Use these questions as a guide; however, you are also free to give additional project feedback on anything that may have been overlooked with the questions listed above. Feedback is due by the end of the session. Please save your feedback as a file or gist and send it to your instructors and the pair. Remember to follow the feedback guidelines you went over as part of your professional development in Mod 1.
We agree that the methods guessLetter, solvePuzzle, and spinWheel could probably live in the Game class. The buyVowel method we assume lives in Player because it is subtracting $100 -- so it depends what you want to do with that method. Within our game class, we have similar methods to the 3 listed above, and our methods that live inside player are only those that modify the player's properties.
We have also been trying to figure out how to populate the current puzzle onto the game board. Each string character has been put into an array, but the conditional logic for where to put it in on the puzzle board hasn't been implemented yet. We would be happy to collaborate with you on this!
We recommend being careful with the domUpdates file, in that it should only affect anything that is displayed on the DOM. Event listeners should probably be in the index.js, which I'm sure you will have many! Some floating functions in the index.js could be moved to classes (ours is set up similarly, but is on the refactor list).
guessLetter() would likely be a method with conditional to iterate over your characters in the answer and, depending on the outcome, perform a certain task. Either move on to the next player, or fill in the board and add the money. We recommend thinking about where you want these methods to live before writing it.
In terms of UI usability, the turn option buttons (spin, buy a vowel, or solve) feel a bit disconnected. It's a little unclear that you have all three of those options each turn, despite the instructions being to the left of said options. Any way you can make it clearer that those are the player's three options each turn (moving the elements around) would really help. Specifically, the wheel, below the options feels the most detached -- its unclear whether or when this is a valid option in regards to its above options.
We really like your tests so far! They look fleshed out for where you are at. We especially liked the tests that utilized the assertion to.be.an.instanceof(). Our recommendation is to utilize hooks on the game-test to instantiate that game before each test, instead of using a line up to create the Game inside of every test.
Lastly, we think the overall comp is visually pleasing and want you to know that! We like the color scheme, background image, and the pop-up for the start screen. The only other UI feedback is mentioned above is repositioning some elements. We also just noticed that the vowels and consonants are grouped together which may also be a little confusing based on what a player can actually do based on their turn option. Also, your code looks generally pretty organized including the CSS.
Awesome!!