Rachael Water Class#48
Conversation
beccaelenzil
left a comment
There was a problem hiding this comment.
JS Adagrams
Major Learning Goals/Code Review
| Criteria | yes/no, and optionally any details/lines of code to reference |
|---|---|
Correctly uses variables, and only uses const and let variables. The program prefers const variables. The program never uses var. |
const should be used unless the variable is re-assigned. |
| Practices best-practices in JavaScript syntax. There are semi-colons at the end of most lines that need semi-colons. Variables and functions are named with camelCase. | ✔️ |
| Correctly creates and calls functions within an object with proper syntax (parameters, return statements, etc.) | ✔️ |
| Uses correct syntax for conditional logic and iteration | ✔️ |
| Practices git with at least 3 small commits and meaningful commit messages | ✔️ |
Utilizes unit tests to verify code; tests can run using the command $ npm test test/adagrams.test.js and we see test successes and/or failures |
✔️ |
Functional Requirements
| Functional Requirement | yes/no |
|---|---|
For the drawLetters function, there is an appropriate data structure to store the letter distribution. (You are more likely to draw an 'E' than an 'X'.) |
✔️ |
Utilizes unit tests to verify code; all tests for drawLetters and usesAvailableLetters pass |
✔️ |
Utilizes unit tests to verify code; all tests for scoreWord pass |
✔️ |
Utilizes unit tests to verify code; all tests for highestScoreFrom pass |
This function is not complete. |
Overall Feedback
Good work on this project. In this project you’ve taken some interesting logic and worked it into JavaScript syntax. I've left some in-line comments on ways you might consider refactoring to simplify your logic. Please let me know if you have any questions. It would be great to walk through the logic of the shuffle method and talk through the highestScoreFrom method.
| Overall Feedback | Criteria | yes/no |
|---|---|---|
| Green (Meets/Exceeds Standards) | 5+ in Code Review && 3+ in Functional Requirements | ✔️ |
Code Style Bonus Awards
Was the code particularly impressive in code style for any of these reasons (or more...?)
| Quality | Yes? |
|---|---|
| Perfect Indentation | ✅ |
| Descriptive/Readable | ✅ |
| Logical/Organized | ✅ |
| }; | ||
|
|
||
| // making the pool of all letters. two loops? taken off of slackoverflow but was the "bad way" because it scales poorly. | ||
| let allLetters = [] |
There was a problem hiding this comment.
Not that even though allLetters is augmented, it is never reassigned, so this should be a const
|
|
||
| return array; | ||
| } | ||
| let shuffledLetters = shuffle(allLetters); |
There was a problem hiding this comment.
| let shuffledLetters = shuffle(allLetters); | |
| const shuffledLetters = shuffle(allLetters); |
| } | ||
| } | ||
| // console.log(allLetters) | ||
| // fully stolen from stackoverflow and I only half understand what's happening in it |
There was a problem hiding this comment.
It's ok to use someone else's code, but you should site the source by pasting the link as a comment. Let's talk through this method together.
| drawLetters() { | ||
| // Implement this method for wave 1 | ||
| shuffle(allLetters); | ||
| let letterHand = allLetters.slice(0,10); |
There was a problem hiding this comment.
| let letterHand = allLetters.slice(0,10); | |
| const letterHand = allLetters.slice(0,10); |
| i += 1 | ||
| } | ||
| else { | ||
| results.push(false) |
There was a problem hiding this comment.
Since you are using a for...in loop, you can return out of the loop. As such, consider just return false so that you return out of the function as soon as you find a letter that's not in the hand.
|
|
||
| }, | ||
| scoreWord(word){ | ||
| let scoreChart = {1: ['A', 'E', 'I', 'O', 'U', 'L', 'N', 'R', 'S', 'T'], |
There was a problem hiding this comment.
This works! Consider how you could write the code if you use an object with keys that were letters and values that were the score for each letter.
| }, | ||
| highestScoreFrom(words){ | ||
| let scoreObject = {} | ||
| for (let i = 0; i < words.length; i++) { |
There was a problem hiding this comment.
Considering using for...of to loop through the words in the array.
| for (let i = 0; i < words.length; i++) { | |
| for (let word of words) { |
Assignment Submission: JS Adagrams
Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.
Reflection