C22-Phoenix-Marjana Khatun #26
Conversation
kelsey-steven-ada
left a comment
kelsey-steven-ada
left a comment
There was a problem hiding this comment.
Nice work translating Adagrams to Javascript! Let me know if you have questions on any feedback =]
| let letters = Object.keys(letterPool); | ||
| let randomIndex = Math.floor(Math.random() * letters.length); |
There was a problem hiding this comment.
This implementation has the same distribution issue that I mentioned in the Python version where there is an equal chance to pick any letter since we are choosing from the keys. Even though the tests pass, this doesn't meet what the function description is asking for, so I would like you to revisit this if you have time.
| for (let letter of input) { | ||
| const index = lettersCopy.indexOf(letter); | ||
| if (index != -1) { | ||
| lettersCopy.splice(index, 1); | ||
| } else { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
This implementation has an O(n^2) time complexity, how could we use a frequency map to bring the time complexity down to O(n)?
| } | ||
| let totalScore = 0; | ||
| word = word.toUpperCase(); | ||
| for (let i = 0; i < word.length; i++) { |
There was a problem hiding this comment.
We could simplify the syntax inside the loop a little if we use a for-of loop to iterate directly over the letters in the parameter word:
for (const letter of word) {| } else if (word.length === 10) { | ||
| highestWord = word; | ||
| } else if (word.length < highestWord.length) { | ||
| highestWord = word; | ||
| } |
There was a problem hiding this comment.
Could we combine these cases or do something else to reduce duplication of updating the highestWord variable?
2 participants
No description provided.