Skip to content

Conversation

@J-J-D
Copy link

@J-J-D J-J-D commented Jun 7, 2022

No description provided.

@J-J-D
Copy link
Author

J-J-D commented Jun 7, 2022

Otters - Jodi Denney

Copy link

@goeunpark goeunpark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, Jodi! 🎉

Your tests all pass and this implementation is production-worthy code! You have a clear understanding of const and let and constant variables in JS, your for loops shine, and your semicolons are used with consistency! This project is a Green. 🟢

Keep up the great work! 💖

const correct = { word: "XXXX", score: scoreWord("XXXX") };

throw "Complete test by adding an assertion";
expect(highestScoreFrom(words)).toEqual(correct);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@@ -1,15 +1,123 @@
// import { LETTER_POOL } from "../test/adagrams.test";
const LETTER_POOL = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great use of the JS constant variable here at the top of the file! ✨

Comment on lines +63 to +68
// Build a letterBank array where each letters appearance is accurately weighted
for (const [key, count] of Object.entries(LETTER_POOL)) {
const start = letterBank.length;
letterBank.length += count;
letterBank.fill(key, start, letterBank.length);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rad! This totally works and I learned about Object.entries() syntax from this bit of code so thank you! Another way we could go about this:

for (const letter in LETTER_POOL) {
    for (let i = 0; i < LETTER_POOL[letter]; i++) {
        letterBank.push(letter) 
    }
}

Comment on lines +70 to 76
for (let i = 0; i < 10; i++) {
let temp = Math.floor(Math.random() * letterBank.length);
hand.push(letterBank[temp]);
letterBank.splice(temp, 1);
}
return hand;
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmmm, love the JS syntax for randomly picking 10 numbers (not really but this is probably the best way we can do this)!


export const usesAvailableLetters = (input, lettersInHand) => {
// Implement this method for wave 2
input = input.toUpperCase();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice way to standardize the input! 💯

export const usesAvailableLetters = (input, lettersInHand) => {
// Implement this method for wave 2
input = input.toUpperCase();
let handCopy = [...lettersInHand];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delicious destructuring on this line! 👨🏻‍🍳

for (let i = 0; i < word.length; i++) {
score += LETTER_SCORE[word[i]];
}
if (word.length > 6) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost! The instructions for Wave 3 says If the length of the word is 7, 8, 9, or 10, then the word gets an additional 8 points so we should add a second conditional here to get the right range!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point but if you tried to pass on any word with more than ten letters you wouldnt have made it past UsesAvailableLetters and your dirty cheater word would have ben declared invalid prior to scoring. So setting the upper limit at 10 here is redundant and I saved at least six whole key strokes! Thank you for all the kind words. Made me smile.

return score;
};

export const highestScoreFrom = (words) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great logic for highestScoreFrom! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants