-
Notifications
You must be signed in to change notification settings - Fork 112
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
Sea Turtles JS-Adagrams -Tyrah G. #103
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,15 +1,79 @@ | ||||||
import { SCORE_CHART, LETTER_POOL } from "constants"; | ||||||
|
||||||
//creates letter pool | ||||||
export const createLetterPool = () => { | ||||||
let letterPool = []; | ||||||
for (const [key, value] of Object.entries(LETTER_POOL)) { | ||||||
for (let i = 0; i < value; i++) { | ||||||
letterPool.push(key); | ||||||
} | ||||||
} | ||||||
return letterPool; | ||||||
}; | ||||||
|
||||||
//shuffles letter pool | ||||||
export const drawLetters = () => { | ||||||
// Implement this method for wave 1 | ||||||
let letterPool = createLetterPool(); | ||||||
let letterPoolDeepCopy = JSON.parse(JSON.stringify(letterPool)); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you build a new `letterPool' whenever this function is called, there's no need to make another copy. |
||||||
function shuffleArray(letterPoolDeepCopy) { | ||||||
for (let i = 0; i < letterPoolDeepCopy; i--) { | ||||||
const j = Math.floor(Math.random() * (i + 1)); | ||||||
[array[i], array[j]] = [array[j], array[i]]; | ||||||
} | ||||||
} | ||||||
Comment on lines
+18
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is defined, but never called. As a result, every hand we get is always A A A A A A A A A B. We now have scrabble for 🐑! 😁 I'd suggest moving this out of this function since it doesn't really need to be defined inside the |
||||||
let letterBank = letterPoolDeepCopy.slice(0, 10); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a great way to get the first 10 items out of the list. We can declare |
||||||
return letterBank; | ||||||
}; | ||||||
|
||||||
export const usesAvailableLetters = (input, lettersInHand) => { | ||||||
// Implement this method for wave 2 | ||||||
for (let letters = 0; letters < input.length; letters++) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer a singular variable name. |
||||||
let char = input[letters]; | ||||||
if (lettersInHand.includes(char) === false) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer to avoid explicit comparisons to true or false. Here we would write this as if the hand does NOT include char if (! lettersInHand.includes(char)) { |
||||||
return false; | ||||||
} else if (lettersInHand.includes(char)) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Either a letter is included or not, so this could be an else clause rather than else if
Suggested change
|
||||||
lettersInHand.splice(char[letters], 1); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't working properly. It appears to because the letters in the word and hand apparently line up in the tests. But really, We can see this with this example usesAvailableLetters('cat', ['t', 'a', 'c']) which should be true, but returns false. So we would need to find the position of a letter to splice out of the array before doing the splice. Check out Also, we don't want to modify that hand that was passed in. So if we're doing to take a destructive (to the hand) approach here, we would want to make a local copy before modifying it. |
||||||
} | ||||||
} | ||||||
return true; | ||||||
}; | ||||||
|
||||||
export const scoreWord = (word) => { | ||||||
// Implement this method for wave 3 | ||||||
let score = 0; | ||||||
//iterate through DOG | ||||||
for (let char = 0; char < word.length; char++) { | ||||||
let input = word[char]; | ||||||
if (input.toUpperCase() in SCORE_CHART) { | ||||||
score += SCORE_CHART[input.toUpperCase()]; | ||||||
} | ||||||
Comment on lines
+45
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice thought to add this bit of additional safety. It wasn't a requirement, though in the context of these functions, we might expect to only receive words that are made of valid letter tiles. Often, when there are a bunch of functions working together in a larger system, we'll expect the validation to have been at one central point so that we don't need to think about so many edge cases in the rest of our code. But nice overall approach of iterating over the word, looking up the score of each letter, and adding the bonus if needed. |
||||||
} | ||||||
if (word.length > 6 && word.length < 11) { | ||||||
score += 8; | ||||||
} | ||||||
return score; | ||||||
}; | ||||||
|
||||||
export const highestScoreFrom = (words) => { | ||||||
// Implement this method for wave 1 | ||||||
let maxScore = 0; | ||||||
let highScoreWord = ""; | ||||||
|
||||||
for (let word = 0; word < words.length; word++) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice job. Your cascading checks are pretty easy to follow! Here, consider a iteration variable name that indicates that this is numeric ( |
||||||
let input = words[word]; | ||||||
let wordScore = scoreWord(input); | ||||||
if (wordScore > maxScore) { | ||||||
maxScore = wordScore; | ||||||
highScoreWord = input; | ||||||
} else if (wordScore === maxScore) { | ||||||
let wordLen = input.length; | ||||||
let highScoreWordLen = highScoreWord.length; | ||||||
{ | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to introduce a block here. |
||||||
if ( | ||||||
highScoreWordLen !== 10 && | ||||||
(wordLen === 10 || wordLen < highScoreWordLen) | ||||||
) { | ||||||
highScoreWord = input; | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
return { score: maxScore, word: highScoreWord }; | ||||||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
export const LETTER_POOL = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice way to avoid cluttering up the logic file with these massive constants! |
||
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, | ||
}; | ||
|
||
export const SCORE_CHART = { | ||
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, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,7 +120,9 @@ describe("Adagrams", () => { | |
}); | ||
|
||
it("returns a score of 0 if given an empty input", () => { | ||
throw "Complete test"; | ||
expectScores({ | ||
"": 0, | ||
}); | ||
Comment on lines
+123
to
+125
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice use of the helper. |
||
}); | ||
|
||
it("adds an extra 8 points if word is 7 or more characters long", () => { | ||
|
@@ -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") }; | ||
|
@@ -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", () => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great way to build up the pool of tiles to draw from.
Since we modify only the contents of
letterPool
, it could still be declared withconst
since we never reassign the variable.