Sabs F Zoisite Adagram.js #80
Conversation
| @@ -1,15 +1,148 @@ | |||
| const LETTER_POOL = { | |||
There was a problem hiding this comment.
Great job using upper case snake case for your global constant!
| // Implement this method for wave 1 | ||
| }; | ||
| let lettersList = []; | ||
| let letterFreq = {}; |
There was a problem hiding this comment.
Because we are never changing the reference to either lettersList or lettersFreq, it might be a good idea to use const for these two! In fact, it's never a bad idea to use const by default. The compiler will catch us if we try to change one!
|
|
||
|
|
||
| while (lettersList.length < 10) { | ||
| let randomLetter = Object.keys(LETTER_POOL)[Math.floor(Math.random() * Object.keys(LETTER_POOL).length)]; |
There was a problem hiding this comment.
We use Object.keys(LETTER_POOL) a couple of times here and it starts to get a bit clunky. Feel free to pull that out into its own variable!
| } | ||
| } | ||
|
|
||
| return lettersList; |
There was a problem hiding this comment.
This looks good, but one think to think about is that it currently weighs all 26 letters the same! If we were to refactor this, see if there's a way we could go ahead and flatten LETTER_POOL into a list that holds every single letter!
| }; | ||
|
|
||
| export const usesAvailableLetters = (word, letterBank) => { | ||
|
|
||
| let bankCopy = letterBank.slice(); |
There was a problem hiding this comment.
Great use of slice to make a shallow copy of letterBank!
|
|
||
| for (let i = 0; i < wordUpper.length; i++) { | ||
| let letter = wordUpper[i]; | ||
| if (!bankCopy.includes(letter) || bankCopy.indexOf(letter) === -1) { |
There was a problem hiding this comment.
Both of these conditionals check to see if the letter is in bankCopy, so I would go ahead and just stick with one!
| } | ||
| } | ||
| return true; | ||
| }; |
There was a problem hiding this comment.
Great use of splice, but the structure of your ifs and elses here could definitely be cleaned up to increase readability!
|
|
||
| if (word.length >= 7) { | ||
| total += 8 | ||
| } |
There was a problem hiding this comment.
This could be turned into a ternary operator!
| total += 8 | ||
| } | ||
|
|
||
| return total; | ||
| }; |
There was a problem hiding this comment.
Overall this function looks good, but make sure to play around with your brackets. Remember that any nested statements should be indented over to increase readability.
|
|
||
| highScoringWords.sort((a, b) => a.length - b.length); | ||
| return { word: highScoringWords[0], score: highestScore } | ||
| }; |
There was a problem hiding this comment.
I understand the concept behind what you are trying to do here! Using objects to separate out each step makes sense, but it starts to get tedious and more complex when we do that. Since we are returning an object with word and score as keys, why not create the one object that has the first word and the first word score as values? From there, we can run through the rest of the wordList and make the comparisons we need in place! This will drastically decrease the complexity and code you need to use! An example is below:
export const highestScoreFrom = (words) => {
let winningWord = {
word: null,
score: null
}
for (let word of words){
let thisWord = {
word: word,
score: scoreWord(word)
}
if (thisWord.score > winningWord.score){
winningWord = thisWord
} else if (thisWord.score === winningWord.score){
const isBestTen = winningWord.word.length === 10;
const isWordTen = thisWord.word.length === 10;
const isWordShorter = thisWord.word.length < winningWord.word.length;
if ((isWordTen && !isBestTen) || (isWordShorter && !isBestTen)){
winningWord = thisWord;
}
}
}
return winningWord;
}
No description provided.