Skip to content
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
2 changes: 2 additions & 0 deletions package.json
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.28.0",
"eslint-plugin-jest": "^27.1.6",
"jest": "^24.8.0",
"regenerator-runtime": "^0.12.1"
},
Expand Down
120 changes: 120 additions & 0 deletions src/adagrams.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,135 @@
// import { keyOf } from "core-js/core/dict";

export const drawLetters = () => {
// Implement this method for wave 1
const letterPool = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider moving this outside the function (or wrap it aggressively) so that the function becomes shorter.

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,
};

let hand = [];
let letters = [];

for (const [key, value] of Object.entries(letterPool)) {
for (let i = 0; i < value; i++) {
letters.push(key);
}
}
while (hand.length < 10) {
hand.push(letters.pop(Math.floor(Math.random() * letters.length - 1)));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In JS, pop always pops from the end. We need to use something like splice to remove from some other position. So this currently always returns the hand

['Z', 'Y', 'Y', 'X', 'W', 'W', 'V', 'V', 'U', 'U']

Talk about hard mode!

}
return hand;
};

export const usesAvailableLetters = (input, lettersInHand) => {
// Implement this method for wave 2
let inputUpper = input.toUpperCase();
let lettersInHandCopy = [...lettersInHand];
for (let letter of inputUpper) {
if (lettersInHandCopy.includes(letter)) {
let indexHand = lettersInHandCopy.indexOf(letter);
lettersInHandCopy.splice(indexHand, 1);
Comment on lines +53 to +55

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Both includesand indexOf require iterating through the data. But indexOf returns -1 if the sought value isn't present. So we could omit the includes check like

    const letterIndex = lettersInHandCopy.indexOf(letter)
    if (letterIndex !== -1) {
      lettersInHandCopy.splice(letterIndex, 1)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Observe that we are performing operations (indexOf and splice) which require iterating through the array of data, and that we are in a loop. So we're iterating over the data each time of the outer loop.

Consider building frequency maps for the input and the hand which requires iterating over each only once. Then rather than repeatedly removing letters from the hand array, we could compare the counts of each input letter with the available count in the hand.

} else {
return false;
}
}
return true;
};

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider moving this outside the function (or wrap it aggressively) so that the function becomes shorter.

A: 1,
B: 3,
C: 3,
D: 2,
E: 1,
F: 4,
G: 2,
H: 4,
I: 1,
J: 9,
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;

for (let letter of word) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Prefer const for the loop variable in a for/of loop.

let value = lettersDict[letter.toUpperCase()];
score += value;
}
if (word.length > 6) {
score += 8;
} else if (word.length == "") {
return 0;
}
Comment on lines +102 to +104

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This condition is unnecessary. If the word is empty, score will be initialized to 0, the loop will not be entered (leaving it 0), and no bonus will be added, still leaving it 0.

return score;
};

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

const wordsDict = {};

for (let word of words) {
wordsDict[word] = scoreWord(word);
}
const winner = { word: words[0], score: wordsDict[words[0]] };

for (const [word, score] of Object.entries(wordsDict)) {
if (score > winner["score"]) {
winner["word"] = word;
winner["score"] = score;
console.log(word, score);
}
if (score === winner["score"] && winner["word"].length < 10) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This should be else if. We don't need to consider this if the word flat out beat the current winner.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Otherwise, nice job teasing apart the tie breakers.

if (word.length == 10) {
winner["word"] = word;
winner["score"] = score;
} else if (word.length < winner["word"].length) {
winner["word"] = word;
winner["score"] = score;
}
}
}
return winner;
};
20 changes: 11 additions & 9 deletions test/adagrams.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,17 @@ describe("Adagrams", () => {
it("does not draw a letter too many times", () => {
for (let i = 0; i < 1000; i++) {
const drawn = drawLetters();
const letter_freq = {};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We didn't really need to update this, but the linter was probably complaining about the case. 😂

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]);
}
}
});
Expand Down Expand Up @@ -120,7 +120,9 @@ describe("Adagrams", () => {
});

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

"": 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 @@ -144,8 +146,8 @@ describe("Adagrams", () => {
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";
// throw "Complete test by adding an assertion";
expect(highestScoreFrom(words)).toEqual(correct);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

});

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