Skip to content

Snow Leopards - Aretta B. #127

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 132 additions & 4 deletions src/adagrams.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,143 @@
export const drawLetters = () => {
// Implement this method for wave 1
const letterPool = {
A: 9,
B: 2,
C: 2,
D: 4,
E: 12,
F: 2,
G: 3,
H: 2,
I: 9,
J: 1,
K: 1,
L: 4,
M: 2,
N: 6,
O: 8,
P: 2,
Q: 1,
R: 6,
S: 4,
T: 6,
U: 4,
V: 2,
W: 2,
X: 1,
Y: 2,
Z: 1,
};



const letters = Object.keys(letterPool);
let hand = [];
while (hand.length < 10) {

Choose a reason for hiding this comment

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

Nice job creating the hand! I'm not seeing where you account for the distribution of letters though. Seems like all letters have an equal chance of being selected.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry to seem like I'm being sensitive over this because I see I you still gave me green on this assignment and I appreciate it, but I really thought I got it right. I even ran to test 10 times in a row to make sure it worked. To my understanding, while const object keys are immutable their values aren't, so every time the loop ran can choose a letter from the list of keys it would go back and subtract from the value of that letter in the letter pool. The letter only got added to the hand if its matching value was above 0. Maybe I didn't do line 37 correctly. Please let me know.

Choose a reason for hiding this comment

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

No problem! The function is getting letters from letterPool just fine, but it isn't accounting for the distribution of letters, so it should be the case that because the letter pool should contain 9 Is and 1 J, that it's 9 times more likely to draw an I. Think of it in an array form, simplified:

['A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'B', 'B',...]

If we generated a random index based on the length of this array, it would be much more likely to be an A. But yours works a bit differently. You generate a number between 1 and 26 and that is used as an index to key off of the letterPool object, so all letters have a 1 in 26 chance of being selected, whereas in the array above, if we created an array that had all the letters according to their distribution, which would produce an array of size 98, we'd end up with a 9/98 chance of drawing an A. As for the decrementing, you're right that you need to decrement the value of each letter in the letter pool to make sure there are available tiles to draw and put into the hand, but there's a mismatch between letters and letterPool which is why the distribution of letters isn't accurate.

let letter = letters[Math.floor(Math.random() * letters.length)];
letterPool[letter] = letterPool[letter] - 1;
if (letterPool[letter] > 0) {
hand.push(letter);
};
};
return hand;
};

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

Choose a reason for hiding this comment

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

Good job with this function!

let hand = [];

for(let letter of lettersInHand) {
hand.push(letter);
};

for(let characters of testWord){
if (hand.includes(characters)) {
hand.splice(characters, 1);
} else {
return false;
};
};

return true
};

export const scoreWord = (word) => {
// Implement this method for wave 3
const scoreChart= {

Choose a reason for hiding this comment

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

This score chart object could be moved into the main scope so it wouldn't be created each time the function scoreWord is called. Also, note the object keys in JavaScript don't need quotes.

'A': 1,
'B': 3,
'C': 3,
'D': 2,
'E': 1,
'F': 4,
'G': 2,
'H': 4,
'I': 1,
'J': 8,
'K': 5,
'L': 1,
'M': 3,
'N': 1,
'O': 1,
'P': 3,
'Q': 10,
'R': 1,
'S': 1,
'T': 1,
'U': 1,
'V': 4,
'W': 4,
'X': 8,
'Y': 4,
'Z': 10
};

let score = 0;
const scoreWord = word.toUpperCase();

for (let letter of scoreWord) {
score += scoreChart[letter]
}

if (word.length >= 7) {
score += 8
}
return score
};

export const highestScoreFrom = (words) => {
// Implement this method for wave 4
const scoredWords = new Map();

Choose a reason for hiding this comment

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

The highestScoreFrom function works great! I'd suggest simplifying the data structure transformations wherever possible, e.g. setting up a Map object isn't doing anything that a plain object can't.


words.forEach(word => {
scoredWords.set(word,scoreWord(word));
});

let scores = scoredWords.values();

Choose a reason for hiding this comment

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

I don't see where these variables are reassigned, so they should be const

let highScore = Math.max(...scores);


const scoredWordsArray = Array.from(scoredWords.entries());
function checkWord(word) {

Choose a reason for hiding this comment

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

Since checkWord is only used in the filter callback, it would be tidier to write the filter function with an inline anonymous arrow function, i.e.

  const highScoreWords = scoredWordsArray.filter(word => word[1] === highscore);

return word[1] === highScore;

}

const highScoreWords = scoredWordsArray.filter(checkWord)


let smallestWordLength = Infinity

Choose a reason for hiding this comment

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

There's some accidental indentation starting here on line 128 through 140.

let previousWord = ["", 0]
highScoreWords.forEach(wordEntry => {
let wordLength = wordEntry[0].length
if (previousWord[0].length === 10)
return //prevents if statement on next line from running
if (wordLength === 10) {
previousWord = wordEntry
} else if (wordLength < smallestWordLength) {
smallestWordLength = wordLength
previousWord = wordEntry
}
})
return {"score": previousWord[1], "word": previousWord[0]}


};
8 changes: 5 additions & 3 deletions test/adagrams.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@ describe("Adagrams", () => {
});

it("returns a score of 0 if given an empty input", () => {
throw "Complete test";
expectScores({
"":0
});
});

it("adds an extra 8 points if word is 7 or more characters long", () => {
Expand All @@ -133,7 +135,7 @@ describe("Adagrams", () => {
});
});

describe.skip("highestScoreFrom", () => {
describe("highestScoreFrom", () => {
it("returns a hash that contains the word and score of best word in an array", () => {
const words = ["X", "XX", "XXX", "XXXX"];
const correct = { word: "XXXX", score: scoreWord("XXXX") };
Expand All @@ -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);

Choose a reason for hiding this comment

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

Nice 👍🏽

});

describe("in case of tied score", () => {
Expand Down