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.

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

G Quinn Kunzite JS Adagrams #83

Open
wants to merge 6 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
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.41.0",
"eslint-plugin-jest": "^27.2.1",
"jest": "^24.8.0",
"regenerator-runtime": "^0.12.1"
},
Expand Down
135 changes: 134 additions & 1 deletion src/adagrams.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,148 @@
export const drawLetters = () => {
// Implement this method for wave 1
const letters= []
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
}
while (letters.length <10){
const keys = Object.keys(letterPool);
const len = keys.length;
const rnd = Math.floor(Math.random() * len);
const key = keys[rnd];

if (letterPool[key] > 0){
letterPool[key]-=1;
}else{
continue;
}


letters.push(key);

Choose a reason for hiding this comment

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

If you moved this into the if block, you wouldn't need the continue. I think that reads better. Also use caution with line spacing (one line space is really the maximum) and remember that there's spacing around the else, e.g.

if (someCond) {
  // do stuff
} else { 
  // do something else
}


}
return letters;


};

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

Choose a reason for hiding this comment

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

Nice, this is a great way to make a shallow copy, which is all we need here!

for (const letter of input){
if (!lettersInHandCopy.includes(letter)){
return false;

Choose a reason for hiding this comment

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

Nice job returning early here!

} const index = lettersInHandCopy.indexOf(letter);

Choose a reason for hiding this comment

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

this const declaration should be on a new line

let temp = lettersInHandCopy[lettersInHandCopy.length - 1]

Choose a reason for hiding this comment

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

temp isn't reassigned so it should be a const, but looking at the next few lines of code, shifting elements in an array around just to use pop(), while it works, is less ideal than using the array method .splice(). You could just use the index you have to to splice out the element from the array like so:

lettersInHandCopy.splice(index, 1);

and skip the element shifting! Remember simpler is almost always better!

lettersInHandCopy[lettersInHandCopy.length - 1] = lettersInHandCopy[index];
lettersInHandCopy[index]=temp;

Choose a reason for hiding this comment

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

leave spaces around assignments:

lettersInHandCopy[index] = temp;

lettersInHandCopy.pop();

}return true;

Choose a reason for hiding this comment

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

this should be on a new line

};

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

const points =[]
const scoreChart = {
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
}

if (!word){
points.push(0)
}

let newWord = word.toUpperCase();

Choose a reason for hiding this comment

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

Since this variable isn't reassigned, you should use const over let.


for (const letter of newWord){
if (letter in scoreChart){
points.push(scoreChart[letter])
}
}
console.log(points)
let total = points.reduce((sum, num) => sum + num);

Choose a reason for hiding this comment

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

Good use of reduce to sum an array of numbers, but consider whether an array is the best data structure (or whether a structure is even necessary. You could just start by assigning total to 0, then in the for-loop on line 107, add the score to total. That approach is more straightforward and doesn't involve the additional iteration that .reduce() does.

console.log(total);

Choose a reason for hiding this comment

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

Logging to the console is a very useful debugging technique, however, we don't want to end up merging code in that contains log statements, so be sure to check your code before submitting a PR to make sure you've removed any debugging console logs.

if (word.length >= 7){
(total += 8);

Choose a reason for hiding this comment

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

This expression shouldn't have parentheses around it

}

return total;
}

export const highestScoreFrom = (words) => {
// Implement this method for wave 4
const scoresDict = {};
let maximum = 0;
let maxKey = null;
let score = 0;

words.sort((a, b) => a.length - b.length);
for (const word of words) {
score = scoreWord(word);
scoresDict[word] = score;
}

for (const scores in scoresDict) {
if (scoresDict[scores] > maximum) {
maximum = scoresDict[scores];
maxKey = scores;
} else if (words[words.length - 1].length === words[words.length - 2].length && score === maximum) {
maxKey = words[words.length - 2];
} else if (words[words.length - 1].length === 10 && score=== maximum) {
maxKey = words[words.length - 1];
}
}
return {word:maxKey, score:maximum};

};

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

Choose a reason for hiding this comment

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

Nice!

});
});

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.

Good job

});

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