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

Nadia Sarkissian Zoisite #79

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

Nadia Sarkissian Zoisite #79

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0e2c432
finish wave 01
pizzagirl1 Jun 5, 2023
ef7726e
wave 02
pizzagirl1 Jun 6, 2023
4160825
refactors wave 02
pizzagirl1 Jun 6, 2023
66ea49c
minor refactoring of waves 1 and 2
pizzagirl1 Jun 7, 2023
889db58
wave 03
pizzagirl1 Jun 7, 2023
eda1647
wave 04
pizzagirl1 Jun 7, 2023
41e2712
final touches
pizzagirl1 Jun 7, 2023
e169e5e
refactors changes with instructor feedback
pizzagirl1 Jun 13, 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
2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
"babel-core": "^7.0.0-bridge.0",
"babel-jest": "^24.8.0",
"babel-plugin-module-resolver": "^3.2.0",
"eslint": "^8.41.0",
"eslint-plugin-jest": "^27.2.1",
"jest": "^24.8.0",
"regenerator-runtime": "^0.12.1"
},
Expand Down
101 changes: 94 additions & 7 deletions src/adagrams.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,102 @@
export const drawLetters = () => {
// Implement this method for wave 1
};
const LETTER_POOL = {
Copy link

Choose a reason for hiding this comment

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

Since this is a local variable of a function, the best practice is to use camel case for the naming: letterPool

Copy link
Author

Choose a reason for hiding this comment

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

Ohh, I kept it in all-caps since we treated it like a constant variable the first time. I wasn't confident about doing all-caps here instead of camelCase.

'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 shuffleArray = array => {
Copy link

Choose a reason for hiding this comment

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

Neat use of a nested function!

for (let i = array.length - 1; i > 0; i--) {
const j = Math.floor(Math.random() * (i + 1));
const temp = array[i];
array[i] = array[j];
array[j] = temp;
}
return array
}

let letterBag = [];
for (const letter in LETTER_POOL) {
for (let i = 0; i < LETTER_POOL[letter]; i++) {
letterBag.push(letter);
}
}
letterBag = shuffleArray(letterBag);
const playerHand = letterBag.slice(0, 10);
return playerHand;
Copy link

Choose a reason for hiding this comment

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

Since this is a pretty short statement and we don't alter playerHand after creating it, I'd consider directly returning letterBag.slice(0, 10);.

}

export const usesAvailableLetters = (input, lettersInHand) => {
// Implement this method for wave 2
};
let letterHashTable = {};
for (let letter of lettersInHand) {
if (letter in letterHashTable){
++letterHashTable[letter];
} else {
letterHashTable[letter] = 1;
}
}

for (let letter of input) {
const letterIsUnavailable = (letterHashTable[letter] === 0)
|| !(letter in letterHashTable);
Copy link

Choose a reason for hiding this comment

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

When dropping a statement across lines, I recommend indenting the continuations one further than the initial line to make it visually clear they are related and the continuations aren't their own new statements.

Copy link
Author

Choose a reason for hiding this comment

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

I always forget what the best way to do the indentation is! I knew this looked funny.


if (letterIsUnavailable) {
return false;
} else {
--letterHashTable[letter];
}
}
return true;
}

export const scoreWord = (word) => {
// Implement this method for wave 3
const LETTER_SCORE = {
'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
};

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

for (let letter of word){
score += LETTER_SCORE[letter];
}

if (word.length >= 7) {
score += 8;
}
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)
score = (word.length >= 7) ? (score + 8) : score

return score;
};

export const highestScoreFrom = (words) => {
// Implement this method for wave 4
};
let winningWord = {
word: null,
score: null
}

for (let word of words){
Copy link

Choose a reason for hiding this comment

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

Great tie breaking in a single loop!

Copy link
Author

Choose a reason for hiding this comment

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

you said the same thing last time! :P

let thisWord = {
word: word,
score: scoreWord(word)
}

if (thisWord.score > winningWord.score){
winningWord = thisWord
} else if (thisWord.score === winningWord.score){
if (winningWord.word.length === 10){
continue;
} else if (thisWord.word.length === 10
|| thisWord.word.length < winningWord.word.length){
winningWord = thisWord;
}
}
Copy link

Choose a reason for hiding this comment

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

Since the result is no change for the first conditional, we could refactor to remove the continue. There are many ways to approach it, one option is to use variables to represent the conditionals we need to consider:

const isBestTen = winningWord.word.length === 10
const isWordTen = thisWord.word.length === 10
const isWordShorter = thisWord.word.length < winningWord.word.length
if ((isWordTen && !isBestTen) || (isWordShorter && !isBestTen)) {
    winningWord = thisWord;
}

Copy link
Author

Choose a reason for hiding this comment

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

i love how you set up these complex conditionals! Thank you for this. My spidey sense started tingling ISO a refactor when I had one line that was just "continue"

}
return winningWord;
}
100 changes: 51 additions & 49 deletions test/adagrams.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
usesAvailableLetters,
scoreWord,
highestScoreFrom,
} from "adagrams";
} from 'adagrams';

const LETTER_POOL = {
A: 9,
Expand Down Expand Up @@ -34,15 +34,15 @@ const LETTER_POOL = {
Z: 1,
};

describe("Adagrams", () => {
describe("drawLetters", () => {
it("draws ten letters from the letter pool", () => {
describe('Adagrams', () => {
describe('drawLetters', () => {
it('draws ten letters from the letter pool', () => {
const drawn = drawLetters();

expect(drawn).toHaveLength(10);
});

it("returns an array, and each item is a single-letter string", () => {
it('returns an array, and each item is a single-letter string', () => {
const drawn = drawLetters();

expect(Array.isArray(drawn)).toBe(true);
Expand All @@ -51,79 +51,81 @@ describe("Adagrams", () => {
});
});

it("does not draw a letter too many times", () => {
it('does not draw a letter too many times', () => {
for (let i = 0; i < 1000; i++) {
const drawn = drawLetters();
const letter_freq = {};
const letterFreq = {};
for (let letter of drawn) {
if (letter in letter_freq) {
letter_freq[letter] += 1;
if (letter in letterFreq) {
letterFreq[letter] += 1;
} else {
letter_freq[letter] = 1;
letterFreq[letter] = 1;
}
}

for (let letter of drawn) {
expect(letter_freq[letter]).toBeLessThanOrEqual(LETTER_POOL[letter]);
expect(letterFreq[letter]).toBeLessThanOrEqual(LETTER_POOL[letter]);
}
}
});
});

describe("usesAvailableLetters", () => {
it("returns true if the submitted letters are valid against the drawn letters", () => {
const drawn = ["D", "O", "G", "X", "X", "X", "X", "X", "X", "X"];
const word = "DOG";
describe('usesAvailableLetters', () => {
it('returns true if the submitted letters are valid against the drawn letters', () => {
const drawn = ['D', 'O', 'G', 'X', 'X', 'X', 'X', 'X', 'X', 'X'];
const word = 'DOG';

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

it("returns false when word contains letters not in the drawn letters", () => {
const drawn = ["D", "O", "X", "X", "X", "X", "X", "X", "X", "X"];
const word = "DOG";
it('returns false when word contains letters not in the drawn letters', () => {
const drawn = ['D', 'O', 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X'];
const word = 'DOG';

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

it("returns false when word contains repeated letters more than in the drawn letters", () => {
const drawn = ["D", "O", "G", "X", "X", "X", "X", "X", "X", "X"];
const word = "GOOD";
it('returns false when word contains repeated letters more than in the drawn letters', () => {
const drawn = ['D', 'O', 'G', 'X', 'X', 'X', 'X', 'X', 'X', 'X'];
const word = 'GOOD';

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

describe("scoreWord", () => {
describe('scoreWord', () => {
const expectScores = (wordScores) => {
Object.entries(wordScores).forEach(([word, score]) => {
expect(scoreWord(word)).toBe(score);
});
};

it("returns an accurate numerical score according to the score chart", () => {
it('returns an accurate numerical score according to the score chart', () => {
expectScores({
A: 1,
DOG: 5,
WHIMSY: 17,
});
});

it("returns a score regardless of the input case", () => {
it('returns a score regardless of the input case', () => {
expectScores({
a: 1,
dog: 5,
wHiMsY: 17,
});
});

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

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

describe.skip("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") };
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') };

expect(highestScoreFrom(words)).toEqual(correct);
});

it("accurately finds best scoring word even if not sorted", () => {
const words = ["XXX", "XXXX", "X", "XX"];
const correct = { word: "XXXX", score: scoreWord("XXXX") };
it('accurately finds best scoring word even if not sorted', () => {
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", () => {
describe('in case of tied score', () => {
const expectTie = (words) => {
const scores = words.map((word) => scoreWord(word));
const highScore = scores.reduce((h, s) => (h < s ? s : h), 0);
Expand All @@ -158,36 +160,36 @@ describe("Adagrams", () => {
expect(tiedWords.length).toBeGreaterThan(1);
};

it("selects the word with 10 letters", () => {
const words = ["AAAAAAAAAA", "BBBBBB"];
it('selects the word with 10 letters', () => {
const words = ['AAAAAAAAAA', 'BBBBBB'];
const correct = {
word: "AAAAAAAAAA",
score: scoreWord("AAAAAAAAAA"),
word: 'AAAAAAAAAA',
score: scoreWord('AAAAAAAAAA'),
};
expectTie(words);

expect(highestScoreFrom(words)).toEqual(correct);
expect(highestScoreFrom(words.reverse())).toEqual(correct);
});

it("selects the word with fewer letters when neither are 10 letters", () => {
const words = ["MMMM", "WWW"];
const correct = { word: "WWW", score: scoreWord("WWW") };
it('selects the word with fewer letters when neither are 10 letters', () => {
const words = ['MMMM', 'WWW'];
const correct = { word: 'WWW', score: scoreWord('WWW') };
expectTie(words);

expect(highestScoreFrom(words)).toEqual(correct);
expect(highestScoreFrom(words.reverse())).toEqual(correct);
});

it("selects the first word when both have same length", () => {
const words = ["AAAAAAAAAA", "EEEEEEEEEE"];
it('selects the first word when both have same length', () => {
const words = ['AAAAAAAAAA', 'EEEEEEEEEE'];
const first = {
word: "AAAAAAAAAA",
score: scoreWord("AAAAAAAAAA"),
word: 'AAAAAAAAAA',
score: scoreWord('AAAAAAAAAA'),
};
const second = {
word: "EEEEEEEEEE",
score: scoreWord("EEEEEEEEEE"),
word: 'EEEEEEEEEE',
score: scoreWord('EEEEEEEEEE'),
};
expectTie(words);

Expand Down
Loading