Jackie Aponte - Zoisite #70
Conversation
| @@ -120,7 +120,7 @@ describe("Adagrams", () => { | |||
| }); | |||
|
|
|||
| it("returns a score of 0 if given an empty input", () => { | |||
| throw "Complete test"; | |||
| expect(scoreWord('')).toEqual(0) | |||
| @@ -145,7 +145,7 @@ describe("Adagrams", () => { | |||
| const words = ["XXX", "XXXX", "X", "XX"]; | |||
| const correct = { word: "XXXX", score: scoreWord("XXXX") }; | |||
|
|
|||
| throw "Complete test by adding an assertion"; | |||
| expect(highestScoreFrom(words)).toEqual(correct); | |||
| 'Z': 1 | ||
| }; | ||
|
|
||
| const alphabet = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; |
There was a problem hiding this comment.
Since this is a constant variable we should name it with capital letters
| const alphabet = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; | ||
|
|
||
| const hand = []; | ||
| let letterPool = JSON.parse(JSON.stringify(LETTER_POOL)); |
There was a problem hiding this comment.
Nice job making a deep copy of LETTER_POOL
Also, letterPool is not reassigned so we should use const instead of let. Always prefer const over let. If you accidentally try to reassign a variable that was declared with const, then the JS interpreter will let you know and you can change it to let.
| let letterPool = JSON.parse(JSON.stringify(LETTER_POOL)); | ||
|
|
||
| while(hand.length < 10) { | ||
| const randomLetter = alphabet[Math.floor(Math.random() * alphabet.length)]; |
There was a problem hiding this comment.
You're grabbing a random letter from alphabet which only has 26 letters. Therefore, your chances of getting 'A' would be 1/26. We should randomly get a letter from a list that has 98 tiles (9 As, 2 Bs, etc), in this case you'd have a 9/98 chance of grabbing an A.
While you do decrement the number of letters in the following if statement, you're still only getting a letter from 26 possible letters instead of 98.
| let index = lettersInHand.indexOf(input[i]); | ||
| lettersInHand.splice(index, 1); |
There was a problem hiding this comment.
The time complexity of indexOf and splice in JS is the same as it is in Python, linear. How could you refactor this method to use an object so that when you look for a letter, you'll have constant time instead of linear?
| for (let i in word) { | ||
| score += LETTER_POINTS[word[i]]; | ||
| } |
There was a problem hiding this comment.
Could use a for-of loop here instead since we don't need i for this logic to work:
for (const letter of word) {
score += LETTER_POINTS[letter]
}| }; | ||
|
|
||
|
|
||
| export const highestScoreFrom = (words) => { |
There was a problem hiding this comment.
There are several variables that should be declared with const instead of let in this method. As you continue writing Javascript, keep an eye out for when you'll actually need let. I tend to always use const until the JS interpreter throws an exception and then will change it to let.
| let highScore = 0; | ||
| let shortestWord = 'xxxxxxxxxx'; | ||
|
|
||
| for (let i in words) { |
There was a problem hiding this comment.
Could we use a for-of loop instead? for (word of words) {}
| } | ||
| } | ||
|
|
||
| for (let i in wordScores) { |
There was a problem hiding this comment.
Prefer to use for-of when we don't need i to access values.
No description provided.