Jessica Hebert -C18 Snow Leopards #129
Conversation
There was a problem hiding this comment.
Nice work on js-adagrams. I called out a couple of places regarding adding white spaces -- even though JS doesn't care, adding white spaces where they should be per the JS style guide, helps our eyes read JS code more easily.
There are a few places where you used let when const should be better since the value wasn't being reassigned. When in doubt, use const and when you try to reassign the variable then Javascript will complain and you can refactor it to let.
I did notice you didn't update the test file which meant two tests were failing. When you see failing tests, be sure to see why they're failing (does your code handle all the edge cases? or do you have to finish filling in a test?). If you look in Learn, you should see the failed tests. Please go back and fill in the test assertions where you see "throw "Complete test by adding an assertion"; and push your changes up again. Thanks!
Overall your code was readable and it seems like you've got a good handle on JS so far!
| const availableLetters = []; | ||
|
|
||
| for (const [letter,frequency] of Object.entries(letterPool)){ | ||
| for (let i = 0;i< frequency;i++){ |
There was a problem hiding this comment.
While JS doesn't care about white spaces, we should add spaces after each semi-colon on line 33.
for (let i = 0; i< frequency; i++)
| const thisLetter = availableLetters[Math.floor(Math.random() * availableLetters.length)] | ||
| hand.push(thisLetter) | ||
| let letterIndex = availableLetters.indexOf(thisLetter) | ||
| availableLetters.splice(letterIndex, 1) |
There was a problem hiding this comment.
Could you remove the letter by using splice with thisLetter instead?
|
|
||
| return hand; |
| // Implement this method for wave 2 | ||
| const upperCase = input.toUpperCase(); | ||
|
|
||
| for (let letter of upperCase) { |
There was a problem hiding this comment.
Do you ever reassign letter in the for loop? It doesn't look like it so we should use const instead of let
| // Implement this method for wave 3 | ||
|
|
||
| let score = 0; | ||
| let bonus = 8; |
There was a problem hiding this comment.
Nice, I like that you put the extra bonus points in a variable. Since it is a constant value that we hardcode to 8 and don't change, we can rename it BONUS.
Also, since you don't reassign bonus, we should use const
| for (let i = 0; i < word.length; i++) { | ||
| score += letterScores[word[i]]; | ||
| } |
There was a problem hiding this comment.
You could also use a for-of loop here to iterate over ever letter in word.
for (letter of word) {
score += letterScores[letter]
}
| highestWord = word; | ||
| }else if (score == highestScore){ | ||
| if(highestWord.length==10){ | ||
| break |
There was a problem hiding this comment.
Do you want to break out of the loop or continue on with the loop?
| let highestWord = ''; | ||
| let highestScore = 0; | ||
|
|
||
| for (let word of words){ |
There was a problem hiding this comment.
Since you're not reassigning word we should use const
No description provided.