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

SL - Presley #132

Open
wants to merge 2 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.28.0",
"eslint-plugin-jest": "^27.1.6",
"jest": "^24.8.0",
"regenerator-runtime": "^0.12.1"
},
Expand Down
162 changes: 158 additions & 4 deletions src/adagrams.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,169 @@

export const drawLetters = () => {
// Implement this method for wave 1
const handOfLetters = [];

const lettersPool = {

Choose a reason for hiding this comment

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

Since this is hardcoded value that shouldn't change (which it can't because you use const), you can rename this variable to LETTERS_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 createPool = function() {
const pool = [];
for (const [letter, quantity] of Object.entries(lettersPool)) {
Comment on lines +35 to +36

Choose a reason for hiding this comment

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

Nice job using const for these variables here

for (let i = 0; i < quantity; i++) {
pool.push(letter);
}
}

return pool
};

const getNum = function(pool) {
const maxLength = pool.length - 1;
const theNum = Math.floor(Math.random() * maxLength);
return theNum
};

const pool = createPool();

while(handOfLetters.length < 10) {
const getTheNum = getNum(pool);
const letterScore = function () {
return pool[getTheNum];
}

handOfLetters.push(letterScore());
pool.splice(getTheNum, 1);
};

return handOfLetters
};

Choose a reason for hiding this comment

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

The helper functions createPool and getNum should be created outside of the function drawLetters, but since they'd all be in the adagrams.js file, your drawLetters function will still be able to access them.

If you want to read more about benefits/disadvantages to nested functions, here's a resource: https://evgenii.info/nested-functions/



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

Choose a reason for hiding this comment

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

Usually we use verbs for variables that refer to a function that does something. You could rename this to upperCaseWord. If you had a custom method that had logic to make a word uppercase different from the built in one you used then turnUpperCase would be great for that.

for (let letter of turnUpperCase) {
if (lettersInHandCopy.includes(letter)) {
const index = lettersInHandCopy.indexOf(letter);
lettersInHandCopy.splice(index, 1);
} else {
return false;
}
}
return true;
};

export const scoreWord = (word) => {
// Implement this method for wave 3
// This wave is still failing one test and I'm stuck on how to get it
// to return 0
Comment on lines +82 to +83

Choose a reason for hiding this comment

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

I ran the tests for scoreWord and they passed on my end. Your logic looks fine. I do see the tests for tied scores failing.


const letterScores = {

Choose a reason for hiding this comment

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

Could also rename this with caps and underscore to indicate it's a hardcoded value that shouldn't be changed, like LETTER_SCORES

'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 totalScore = 0;
if(word === '') {
return totalScore;
}
Comment on lines +115 to +117

Choose a reason for hiding this comment

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

You can get rid of this here because if word is an empty string then no iterating will happen on line 118-120, and then your method returns totalScore which is still 0 (unchanged since none of the logic ran)

for (let letter of word.toUpperCase()) {
totalScore += letterScores[letter];
}
if(word.length > 6){
totalScore += 8;
}

return totalScore;
};

export const highestScoreFrom = (words) => {
// Implement this method for wave 4
const wordScoreLength = [];
for (let word of words) {
const wordDict = {};
wordDict['word'] = word;
wordDict['score'] = scoreWord(word);
wordDict['length'] = word.length;
wordScoreLength.push(wordDict);
}
let maxWords = [];
let maxScore = 0;

for (let wordDict of wordScoreLength) {
if (wordDict['score'] == maxScore) {
maxWords.push(wordDict['word']);
} else if (wordDict['score'] > maxScore) {
maxScore = wordDict['score'];
maxWords = [];
maxWords.push(wordDict['word']);
}
}

if (maxWords.length === 1) {

Choose a reason for hiding this comment

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

I think the issue with the logic for handling ties is here. When I ran the tests, I saw there were times when maxWords.length != 1 (instead it had 2 words in it)

return {
word: maxWords[0],
score: maxScore,
};
}

// From here:
// create a for loop (possibly a for of loop) for word in maxWords
//
// that if the length of word is 10, return word & score
//
// then have an else statement that if the length of word is less than 10
//
// find the word with the minimum length and store it in a let variable
//
// after looping through all the words and finding the shortest word
//
// return that word and the word's score
};
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);
});

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