Skip to content
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.

Sign up for GitHub

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

Madi Jackson #87

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Madi Jackson #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fdf7c04
set up adagrams java
dawnsoapp Jun 3, 2023
e21754b
implemented wave 1 and 2
dawnsoapp Jun 7, 2023
8f84ee8
completed wave 3
dawnsoapp Jun 7, 2023
b17463c
implemented working wordDict
dawnsoapp Jun 8, 2023
0b88904
finished wave 4 with help of kelsey! <3
dawnsoapp Jun 14, 2023
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
1 change: 1 addition & 0 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
159 changes: 155 additions & 4 deletions src/adagrams.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,166 @@
const LETTER_POOL = {
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 LETTER_SCORE = {
Comment on lines +27 to +29
Copy link

Choose a reason for hiding this comment

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

Tiny best practices note: The spacing between data structures/functions and indentation isn't consistent across the file, just something to keep in mind to take a look for before opening PRs.

'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
};
export const drawLetters = () => {
// Implement this method for wave 1
let fullHand = [];
let myHand = [];
for (let [key, value] of Object.entries(LETTER_POOL)){
fullHand.push(...Array(value).fill(key));
}
Comment on lines +60 to +62
Copy link

Choose a reason for hiding this comment

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

Great combination of iteration and the spread operator to create the letter pool to draw from.

while (myHand.length < 10){
let randomChoice = Math.floor(Math.random() * fullHand.length);
myHand.push(fullHand[randomChoice]);
fullHand.splice(randomChoice, 1);
}
return myHand;
};

export const usesAvailableLetters = (input, lettersInHand) => {
// Implement this method for wave 2
let testWord = [];
let wordDict = {};
Comment on lines +72 to +73
Copy link

Choose a reason for hiding this comment

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

We should prefer using const anywhere we don't need to reassign values - Lists & dictionaries are mutable, we can declare them as const and still add new values, we just then can't re-assign the variable to a new array or dictionary.

input.toUpperCase();

for (let letter of input){
Copy link

Choose a reason for hiding this comment

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

Love the thought to use a frequency map here!

if (letter in wordDict){
++wordDict[letter];
} else {
wordDict[letter] === 1;
}
}

for (let [key, value] of Object.entries(lettersInHand)){
Copy link

Choose a reason for hiding this comment

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

While javascript allows this syntax, looking at the tests, lettersInHand is an array of strings. To avoid confusion about the kind of data type we're iterating over and remove variables like key that aren't being used, a for-of loop would be a better fit:

for (let letter of lettersInHand) {
...

if (input.includes(value)){
testWord.push(value);
}
}
let finalWord = testWord.join('');
if (finalWord === input){
Comment on lines +89 to +90
Copy link

Choose a reason for hiding this comment

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

There's a bug caused by these lines that I'd like you to try troubleshooting.

What happens if lettersInHand has more than the exact same count of a letter in input, or the letters occur in a different order in lettersInHand than the order in input? I've written a couple tests you can add to the suite to try this out:

    it("returns true when word contains letters in the drawn letters out of order", () => {
      const drawn = ["G", "D", "O", "X", "X", "X", "X", "X", "X", "X"];
      const word = "DOG";

      const isValid = usesAvailableLetters(word, drawn);
      expect(isValid).toBe(true);
    });

    it("returns true when drawn letters contains duplicates of letters in word", () => {
      const drawn = ["D", "O", "G", "G", "X", "X", "X", "X", "X", "X"];
      const word = "DOG";

      const isValid = usesAvailableLetters(word, drawn);
      expect(isValid).toBe(true);
    });

return true;
} else {
return false;
}
Comment on lines +92 to +94
Copy link

Choose a reason for hiding this comment

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

We could remove the else block here and unindent return false; without changing the function's outcome.

};

export const scoreWord = (word) => {
// Implement this method for wave 3
let scoreBoard = 0;
let bonus = 8;
let myWord = word.toUpperCase();
for (let letter of myWord){
if (letter in LETTER_SCORE){
scoreBoard += LETTER_SCORE[letter];
}
}
if (myWord.length >= 7){
scoreBoard += bonus;
}
Comment on lines +106 to +108
Copy link

Choose a reason for hiding this comment

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

This could be a great place for a ternary operator:

// Ternary operator structure: 
// var = (conditional) ? (value if true) : (value if false)
scoreBoard = (myWord.length >= 7) ? (score + bonus) : score

return scoreBoard;
};

export const highestScoreFrom = (words) => {
// Implement this method for wave 4
let wordDict = {};
let winnersList= {};
let lengthList = {};
Comment on lines +114 to +115
Copy link

Choose a reason for hiding this comment

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

We could remove these dictionaries since it looks like they're only referenced in the commented out code.


for (let letter of words){
Copy link

Choose a reason for hiding this comment

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

In this case we're iterating over a list of words rather than letters, I suggest updating the variable name letter to better reflect what it represents.

let score = scoreWord(letter);
wordDict[letter] = score;
// if (letter.length === 10){
// return (letter, score);
// } else{
// continue;
// }
}
let counter = 0
let contestants = ''
Copy link

Choose a reason for hiding this comment

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

Since contestants will contain a single word, it's a naming convention to use a singular name like contestant to represent that.

for (let [key,value] of Object.entries(wordDict)){
if (value > counter){
counter = value;
contestants = key;
}else if (value === counter){
Copy link

Choose a reason for hiding this comment

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

Small style best practice: we should include a space after a closing brace before any following key words.

if (contestants.length === 10){
continue;
}else if (key.length < contestants.length){
contestants = key;
}else if (key.length === 10){
contestants = key;
}
}
}
return {word: contestants, score: counter};
// const maxValue = wordDict.reduce();

// for (let key of wordDict){
// if (wordDict[key] === (key, maxValue)){
// let draft = (key, maxValue);
// winnersList.push(draft);
// }
// }

// for (let [key, value] in Object.entries(winnersList)){
// let len = key.length;
// lengthList.push(len)
// }

// let tieBreaker = Math.min(lengthList);

// if (winnersList.length > 0){
// for (let winner of winnersList){
// if (winner[0].length === tieBreaker){
// return winner;
// }
// }
// }
};
8 changes: 5 additions & 3 deletions test/adagrams.test.js
View file
Open in desktop
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);
});

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