-
Notifications
You must be signed in to change notification settings - Fork 169
Sapphire - Hannah Chandley Wohllaib #136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…, implimented getRandomInt and drawLetters
… Passed all tests for wave 4.
…ere are 2 tests failing. second time tests are run they all pass. this implies an issue with statefulness
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonderful job, Hannah! 🎉
This submission nails all the learning goals. I'm excited to see your JS in the world wide web!
X: 8, | ||
Y: 4, | ||
Z: 10, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love to see the two constants LETTER_VALUE
and LETTER_POOL
up top! 🧡
const getRandomInt = (max) => { | ||
return Math.floor(Math.random() * max); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great helper function!
const randomLetter = alphabet[getRandomInt(26)]; | ||
const quantity = LETTER_POOL[randomLetter]; | ||
let counter = 0; | ||
|
||
for (let i = 0; i < drawnLetters.length; i++) { | ||
if (drawnLetters[i] === randomLetter) { | ||
counter += 1; | ||
} | ||
} | ||
if (counter < quantity) { | ||
drawnLetters.push(randomLetter); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't quite mimic the action of pulling out a random letter form a bag of properly weighted letters (here, there's an equal chance of pulling out a E
as a Z
due to L28, despite E
being much more likely to be drawn) but it's a good start!
const inputUpper = input.toUpperCase(); | ||
|
||
for (const letter of inputUpper) { | ||
if (lettersInHandMap.has(letter) === true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor to:
if (lettersInHandMap.has(letter)) {
@@ -145,7 +147,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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
const tieBreaker = (potentialWinningWords) => { | ||
const tenLetterWords = []; | ||
const shortestWords = []; | ||
let shortestWordLength = 11 | ||
|
||
for (const word of potentialWinningWords) { | ||
if (word.length === 10) { | ||
tenLetterWords.push(word) | ||
} else if (word.length < shortestWordLength) { | ||
shortestWordLength = word.length; | ||
} | ||
} | ||
|
||
for (const word of potentialWinningWords) { | ||
if (word.length === shortestWordLength) { | ||
shortestWords.push(word); | ||
} | ||
} | ||
|
||
if (tenLetterWords.length > 0) { | ||
return tenLetterWords[0]; | ||
} else { | ||
return shortestWords[0]; | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great tiebreaker logic in a neat helper function!
Thank you!!