Ocelots - Kelly T - #13
Conversation
kelsey-steven-ada
left a comment
kelsey-steven-ada
left a comment
There was a problem hiding this comment.
Looks good! I’ve added some suggestions, let me know if there's anything I can clarify ^_^
| for (let [key,value] of Object.entries(letterCounts)) { | ||
| for (let i = 0; i < value; ++i) { | ||
| letterPool.push(key); | ||
| } | ||
| } |
There was a problem hiding this comment.
Really nice solution for ensuring we get the desired distribution of letters.
I would consider moving the creation of the letterPool variable to directly above this loop to keep it right next to where it gets used.
| const shuffle = function (letterPool) { | ||
| letterPool.sort(() => Math.random() - 0.5); | ||
| } |
There was a problem hiding this comment.
Really neat implementation 😄
| } else { | ||
| wordCount[letter] = 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
Great use of frequency maps.
| if (!(key in letterBankCount)) { | ||
| return false; | ||
| } | ||
| if (wordCount[key] > letterBankCount[key]) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
If the line doesn't get too long, I suggest combining these into one if-statement so we can have one false return.
| word = word.toUpperCase(); | ||
|
|
||
| let points = 0; |
There was a problem hiding this comment.
I would consider moving these lines to below scoreChart, or at least the initialization of points so they are right next to where they get used. This can make reading easier since there's no need to scroll if you have to remind yourself of what a value was initialized as.
| } | ||
|
|
||
| // Calculate score for each word | ||
| for (const letter of word) { |
There was a problem hiding this comment.
Nice use of a for...of loop.
| for (const letter of word) { | ||
| points += scoreChart[letter]; | ||
| } |
There was a problem hiding this comment.
This could have an unexpected result if the string word has characters that aren't in scoreChart. If we have an input like "ABC DEF" then points will hold NaN (Not a Number) after the loop. If we wanted to skip non-alphabetic characters or characters that aren't in scoreChart, how could we do that?
| let winner; | ||
|
|
||
| // Determine highestScore and winner | ||
| for (const wordEntry in scores) { |
There was a problem hiding this comment.
Nice implementation to tie break in a single loop!
2 participants
No description provided.