Skip to content

Conversation

@HienXuanPham
Copy link

No description provided.

Copy link

@kelsey-steven-ada kelsey-steven-ada left a comment

Choose a reason for hiding this comment

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

Looks good! I’ve added some suggestions, let me know if there's anything I can clarify ^_^

Z: 1,
};

const scoreChart = {

Choose a reason for hiding this comment

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

Great use of const! Since these constants at the top of the file are only accessed by one function each, it would be reasonable to place the objects inside the functions rather than as a constant. There are tradeoffs, the structure would clutter the function some, but it keeps the data as close as possible to where it's being used, and would mean that other functions who shouldn't need it couldn't access it.

As a constant outside of a function, I suggest using all caps naming: SCORE_CHART

let randomLetter = String.fromCharCode(randomNum + 65);

if (
letterFrequency.hasOwnProperty(randomLetter) &&

Choose a reason for hiding this comment

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

Neat way to check if a key exists! We could also use in like

if (randomLetter in letterFrequency 
        && letterFrequency[randomLetter] < LETTER_POOL[randomLetter]) {
...
}

let letterFrequency = {};
let aHandOfLetters = [];
while (aHandOfLetters.length < 10) {
let randomNum = Math.floor(Math.random() * 26);

Choose a reason for hiding this comment

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

This will give us an even chance of picking any single letter in the alphabet without going over the number of each tile we have. This is slightly different than what the README asks - we won't accurately represent the distribution of tiles because we pick a letter from 1-26, when the chances of picking some letters should be higher than others. How could we update the algorithm to account for this?

Comment on lines +82 to +90
for (const char of input) {
for (const i in lettersInHand) {
if (char === lettersInHand[i]) {
lettersInHand[i] = "";
found++;
break;
}
}
}

Choose a reason for hiding this comment

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

Neat approach! If we needed to reduce the time complexity of our solution, another approach could be to build a frequency map of our hand then loop over the characters in the input, checking if the character is in our frequency map, and if it is, then check the value to see if there are still tiles left in our hand for that letter.

export const scoreWord = (word) => {
// Implement this method for wave 3
let totalPoints = 0;
for (const char of word) {

Choose a reason for hiding this comment

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

Nice use of a for...of loop.

Comment on lines +99 to +101
for (const char of word) {
totalPoints += scoreChart[char.toUpperCase()];
}

Choose a reason for hiding this comment

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

This could have an unexpected result if the string word has characters that aren't in scoreChart. If we have an input like "ABC DEF" then totalPoints will hold NaN (Not a Number) after the loop. If we wanted to skip non-alphabetic characters or characters that aren't in scoreChart, how could we do that?

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