Ada C22 Phoenix Class, Madina Dzhetegenova #42
Conversation
| const SCORE_CHART = {A: 1, | ||
| E: 1, | ||
| I: 1, |
There was a problem hiding this comment.
If the values in the object are spread across multiple lines, list the first value indented along with the remaining values. And since JS doesn't care what order the keys are listed, it would be easier for another coder reading this if the letters were written alphabetically, as it makes it easier to confirm that all the letters are present.
const SCORE_CHART = {
A: 1,
B: 3,
C: 3,| for (const [letter, count] of Object.entries(LETTER_POOL)) { | ||
| for (let i = 0; i < count; i++) { | ||
| letterPool.push(letter); | ||
| } | ||
| } |
There was a problem hiding this comment.
Nice logic to build a pile of all the tiles.
| for (let i = 0; i < 10; i++) { | ||
| const randomIndex = Math.floor(Math.random() * letterPool.length); | ||
| handBank.push(letterPool[randomIndex]); | ||
| letterPool.splice(randomIndex, 1); |
There was a problem hiding this comment.
Much like pop/remove in python, splice will need to shift values forward to close the hole left by the removed item. To avoid the internal loop this requires, we can first swap it with value to the end of the pool, then pop it. Strictly speaking, it's not even necessary to remove the value from the list, so lang as we adjusted the value we used as the list length to reflect the size of the available portion of the pool.
| expectScores({ | ||
| '': 0, | ||
| }); |
| @@ -133,7 +135,7 @@ describe("Adagrams", () => { | |||
| }); | |||
| }); | |||
|
|
|||
| describe.skip("highestScoreFrom", () => { | |||
| describe("highestScoreFrom", () => { | |||
| letter = letter.toUpperCase(); | ||
|
|
||
| if (!lettersInHand.includes(letter)) { | ||
| return false; |
There was a problem hiding this comment.
It looks like your editor switched to 4 space indentations occassionally. For JS, we tend to prefer smaller indentations, since more indented code tends to be common in JS. In Python, we would use deeper indentation as a hint that maybe we should do some refactoring, but that tends not to be as true in JS.
|
|
||
| const index = lettersInHand.indexOf(letter); | ||
| lettersInHand.splice(index, 1); | ||
| } |
There was a problem hiding this comment.
Nit: This closing brace became too unindented.
| let scoreMax = 0; | ||
| let wordMin = "ZZZZZZZZZZZZZZZZZ"; | ||
| let wordsPosWin = []; | ||
|
|
||
| for (let word of words) { | ||
| const wordScore = scoreWord(word); | ||
| if (wordScore > scoreMax) { | ||
| scoreMax = wordScore; | ||
| } | ||
| } | ||
|
|
||
| for (let word of words) { | ||
| const wordScore = scoreWord(word); | ||
| if (wordScore === scoreMax) { | ||
| wordsPosWin.push(word); | ||
| } | ||
| } | ||
|
|
||
| if (wordsPosWin.length === 1) { | ||
| return {word: wordsPosWin[0], score: scoreMax}; | ||
| } | ||
|
|
||
| for (let word of wordsPosWin) { | ||
| if (word.length === 10) { | ||
| return {word: word, score: scoreMax}; | ||
| } | ||
|
|
||
| if (word.length < wordMin.length) { | ||
| wordMin = word; | ||
| } | ||
| } | ||
| return {word: wordMin, score: scoreMax}; | ||
|
|
There was a problem hiding this comment.
This whole function became intended too deeply for some reason and is using 4 space indentations.
| let wordMin = "ZZZZZZZZZZZZZZZZZ"; | ||
| let wordsPosWin = []; | ||
|
|
||
| for (let word of words) { |
There was a problem hiding this comment.
For pretty much any for/of loop, prefer const (here and throughout)
| for (let word of words) { | |
| for (const word of words) { |
|
|
||
| export const highestScoreFrom = (words) => { | ||
| // Implement this method for wave 4 | ||
| let scoreMax = 0; | ||
| let wordMin = "ZZZZZZZZZZZZZZZZZ"; |
There was a problem hiding this comment.
Rather than picking an aribtrary "min" word, if our logic gets to the point where we need to distinguich between multiple tied score words, we can pick the first word in the list of values as the word to beat. This can be handy when it's not clear what a good "min" value is. Even in this case, we have to observer that the good min word will be long (since shorter is better) but NOT have a length of exactly ten letters (since that would actually be better than any following word).
No description provided.