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

Zoisite - Whitney Shake #67

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

Zoisite - Whitney Shake #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8d2f17e
Setup complete
WhitShake May 31, 2023
2cd62a7
Wave 1 passing tests
WhitShake Jun 3, 2023
6194fbb
Reworked some logic for wave 01, all tests passing
WhitShake Jun 3, 2023
8844364
Reworked Wave 01 logic, all tests passing
WhitShake Jun 3, 2023
56e9a36
Refactor wave 01, tests passing
WhitShake Jun 3, 2023
8a82f87
Forgot to switch branches, no changes, recommiting for safety
WhitShake Jun 4, 2023
6026de2
Wave 03 complete, all tests passing
WhitShake Jun 4, 2023
3850d05
All features working perfectly in Replit, not trasferring to VSCode
WhitShake Jun 6, 2023
e3f10ec
Wave 04 passing all tests, ready for refactor
WhitShake Jun 6, 2023
46c6dc3
First pass at refactoring, all tests passing
WhitShake Jun 7, 2023
2c25eec
Final Push, Adagrams Completed
WhitShake Jun 7, 2023
fb5ca8a
Fixed error found during submission
WhitShake Jun 7, 2023
19d5721
For real the final push, All tests and demo tests passing
WhitShake Jun 7, 2023
bf264f4
Merging wave_02
WhitShake Jun 7, 2023
12f5b86
Merge branch 'wave_03'
WhitShake Jun 7, 2023
fed0d44
merged wave_03 and wave_04
WhitShake Jun 7, 2023
8883a96
Had issue merging, corrected
WhitShake Jun 7, 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
324 changes: 320 additions & 4 deletions src/adagrams.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,331 @@
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.

Nice job naming constant variables with all caps

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 SCORE_BOARD = {
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,
};


export const drawLetters = () => {
// Implement this method for wave 1
let letterArray = Object.keys(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 Object.key() returns a list of the keys from LETTER_POOL, letterArray is a list of 26 letters (A-Z). However, to accurately capture the distribution represented by LETTER_POOL, letterArray should be 98 letters (A, A, A, A, A, A, A, A, A, B, B, etc...).

When you randomly get a letter from letterArray on line 25, your chance of drawing an 'A' is 1 of 26, but it should be 9 of 98. How can you use LETTER_POOL to create a list with 98 letters?

Maybe something like:

for (const [key, value] of Object.entries(LETTER_POOL)) {
    for (let i = 0; i < value; i++) {
      letterArray.push(key);
    }
  }

let tempLetters = {};
let lettersInHand = [];
Comment on lines +19 to +21
Copy link

Choose a reason for hiding this comment

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

These values are not re-assigned and should be declared with const. I tend to define everything with const and then switch them to let when the JS interpreter complains.


while (lettersInHand.length < 10) {
let newNum = Math.floor(Math.random() * (26 + 1));
Copy link

Choose a reason for hiding this comment

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

instead of hardcoding 26, prefer to use a variable that describes what the value is or you could use letterArray.length too.

Also, newNum is not re-assigned in the while loop so you should use const

let currentLetter = letterArray[newNum];
Copy link

Choose a reason for hiding this comment

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

use const instead

if (currentLetter === undefined) {
continue;
}
Comment on lines +26 to +28
Copy link

Choose a reason for hiding this comment

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

Instead of writing this condition and then just using continue, maybe you could add another condition to line 29 and then remove lines 26-28 completely.

So line 29 could be like:
if (currentLetter !== undefined and (!currentLetter in tempLetters)) {}

else if (!(currentLetter in tempLetters)) {
tempLetters[currentLetter] = 1;
lettersInHand.push(currentLetter);
}
else if (currentLetter in tempLetters) {
if (tempLetters[currentLetter] < LETTER_POOL[currentLetter]) {
tempLetters[currentLetter] += 1;
lettersInHand.push(currentLetter);
}
}
else {
continue;
Comment on lines +39 to +40
Copy link

Choose a reason for hiding this comment

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

Do we need this?

}
}
return lettersInHand;
};

export const usesAvailableLetters = (input, lettersInHand) => {
// Implement this method for wave 2
let tempLetters = getNewObject(lettersInHand);
let inputObject = getNewObject(input);
Comment on lines +47 to +48
Copy link

Choose a reason for hiding this comment

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

Use const instead of let.

Looking at the implementation of getNewObject, it seems that method returns an object that represents the frequency of letters. It would make your code more readable to name the function more descriptively, maybe something like getLettersFrequencyObject or something else that helps describe what the method does.

tempLetters doesn't describe that the variable references a value the way that inputObject does. Consider renaming tempLetters to something like lettersInHandObject

for (let i = 0; i < input.length; i++) {
Copy link

Choose a reason for hiding this comment

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

Since we don't need i to access every letter in input, prefer to use the for-of loop.

for (const currentLetter in input) {
   if (currentLetter in tempLetters) {
         // your logic here
   }
}

let currentLetter = input[i];
Copy link

Choose a reason for hiding this comment

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

Use const

if (currentLetter in tempLetters) {
if (inputObject[currentLetter] <= tempLetters[currentLetter]) {
continue;
}
else {
return false;
}
}
else {
return false;
}
}
return true;
};
Comment on lines +52 to +64
Copy link

Choose a reason for hiding this comment

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

It's a little hard to understand what your conditional statements are doig here.

How could you simplify the logic here so it's not so nested? I think you can write the conditional statements such that you wouldn't need continue or to have two else statements return false.

when you find yourself checking for some if condition and then continuing, could you invert the condition being checked so you don't need to use continue?

for (let i = 0; i < input.length; i++) {
    let currentLetter = input[i];
    if (!(currentLetter in tempLetters)) {
      return false;
    } else if (tempLetters[currentLetter] < inputObject[currentLetter] ) {
      return false;
    }
  }
  return true;
};


const getNewObject = (letters) => {
let newObject = {}
Copy link

Choose a reason for hiding this comment

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

Use const

Also, consider renaming newObject to something more descriptive like letterFrequencyObject

for (let i = 0; i < letters.length; i++) {
Copy link

Choose a reason for hiding this comment

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

Could we use a for-of loop instead since we don't need i to access each letter in letters?

let currentLetter = letters[i];
Copy link

Choose a reason for hiding this comment

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

use const

if (!(currentLetter in newObject)) {
newObject[currentLetter] = 1;
}
else {
newObject[currentLetter] += 1;
}
}
return newObject;
};

export const scoreWord = (word) => {
// Implement this method for wave 3
let score = 0;
let capsWord = word.toUpperCase();
Copy link

Choose a reason for hiding this comment

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

Use const


for (let i = 0; i < word.length; i++) {
Copy link

Choose a reason for hiding this comment

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

Could we use for-of loop instead?

if (word === "") {
return 0;
}
Comment on lines +85 to +87
Copy link

Choose a reason for hiding this comment

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

You don't need lines 85-87. If word is an empty string then the looping won't happen and the if statement on line 85 will be false. Finally the code will get to line 88 where score is still 0 and you can just return score without needing this extra check

let currentLetter = capsWord[i];
score += SCORE_BOARD[currentLetter];
}
if (word.length >= 7) {
score += 8;
}
return score;
};

export const highestScoreFrom = (words) => {
// Implement this method for wave 4
let scoreArray = getScoreArray(words);
Copy link

Choose a reason for hiding this comment

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

Most of the variables in this method should be declared with const since they are not reassigned.

let highScore = getHighScore(scoreArray);
let potentialWinners = getPotentialWinners(words, scoreArray, highScore);
let tenLetterWinner = checkForTen(potentialWinners);
if (tenLetterWinner) {
return {"word": tenLetterWinner, "score": highScore};
}
else {
let lengthArray = getLengthArray(potentialWinners);
let tieBreaker = getTieBreaker(potentialWinners, lengthArray);
return {"word": tieBreaker, "score": highScore};
}
};
Copy link

Choose a reason for hiding this comment

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

👍 I like the helper functions you wrote to use in highestScoreFrom. The tie breaking logic is pretty complex so breaking each part of the logic into helper functions keeps this function concise and readable.


const getScoreArray = (words) => {
let scoreArray = []
for (let i = 0; i < words.length; i++) {
let currentWord = words[i];
let currentScore = scoreWord(currentWord);
Comment on lines +113 to +116
Copy link

Choose a reason for hiding this comment

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

Use const to declare your variables and for-of loop

scoreArray.push(currentScore);
}
return scoreArray;
};

const getHighScore = (scoreArray) => {
let highScore = Math.max(...scoreArray);
return highScore;
};
Comment on lines +122 to +125
Copy link

Choose a reason for hiding this comment

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

I think that helpers boost readability but when your helper is just one line, you could consider directly calling Math.max in highestScoreFrom on line 99


const getPotentialWinners = (words, scoreArray, highScore) => {
Copy link

Choose a reason for hiding this comment

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

This method relies on the words array and the scoreArray to be the same length since they both use i to access the values. However relying on this fact and that the words and scores are in the same order can be tricky. If you have two lists holding words and their scores, could you use an object to associate words with their scores instead?

You can still iterate over the object to find the potentialWinners

let potentialWinners = [];
for (let i = 0; i < scoreArray.length; i++) {
let winner = scoreArray[i];
let winningWord = words[i];
Comment on lines +128 to +131
Copy link

Choose a reason for hiding this comment

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

Use const

if (winner === highScore) {
potentialWinners.push(winningWord);
}
}
return potentialWinners;
};

const checkForTen = (potentialWinners) => {
for (let i = 0; i < potentialWinners.length; i++) {
let currentWord = potentialWinners[i];
Comment on lines +140 to +141
Copy link

Choose a reason for hiding this comment

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

Use for-of loop and const

if (currentWord.length === 10) {
return currentWord;
}
}
};

const getLengthArray = (potentialWinners) => {
let lengthArray = []
for (let i = 0; i < potentialWinners.length; i++) {
let currentWord = potentialWinners[i].length;
Comment on lines +149 to +151
Copy link

Choose a reason for hiding this comment

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

use const and for-of loop

lengthArray.push(currentWord);
}
return lengthArray;
};

const getTieBreaker = (potentialWinners, lengthArray) => {
for (let i = 0; i < lengthArray.length; i++) {
let currentWord = potentialWinners[i];
let shortestLength = Math.min(...lengthArray);
Comment on lines +158 to +160
Copy link

Choose a reason for hiding this comment

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

Use for-of loop and const where variables are not re-assigned

if (currentWord.length === shortestLength) {
return currentWord;
}
}
};

// const LETTER_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 SCORE_BOARD = {
// 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,
// };


// export const drawLetters = () => {
// let letterArray = Object.keys(LETTER_POOL);
// let tempLetters = {};
// let lettersInHand = [];

// while (lettersInHand.length < 10) {
// let newNum = Math.floor(Math.random() * (26 + 1));
// let currentLetter = letterArray[newNum];
// if (currentLetter === undefined) {
// continue;
// }
// else if (!(currentLetter in tempLetters)) {
// tempLetters[currentLetter] = 1;
// handSelection.push(currentLetter);
// }
// else if (currentLetter in tempLetters) {
// if (tempLetters[currentLetter] < LETTER_POOL[currentLetter]) {
// tempLetters[currentLetter] += 1;
// lettersInHand.push(currentLetter);
// }
// }
// else {
// continue;
// }
// }
// return lettersInHand;
// };

// export const usesAvailableLetters = (input, lettersInHand) => {
// let tempLetters = getNewObject(lettersInHand);
// let inputObject = getNewObject(input);
// for (let i = 0; i < input.length; i++) {
// let currentLetter = input[i];
// if (currentLetter in tempLetters) {
// if (inputObject[currentLetter] <= tempLetters[currentLetter]) {
// continue;
// }
// else {
// return false;
// }
// }
// else {
// return false;
// }
// }
// return true;
// };

// const getNewObject = (letters) => {
// let newObject = {}
// for (let i = 0; i < letters.length; i++) {
// let currentLetter = letters[i];
// if (!(currentLetter in newObject)) {
// newObject[currentLetter] = 1;
// }
// else {
// newObject[currentLetter] += 1;
// }
// }
// return newObject;
// };

// export const scoreWord = (word) => {
// let score = 0;
// let capsWord = word.toUpperCase();

// for (let i = 0; i < word.length; i++) {
// if (word === "") {
// return 0;
// }
// let currentLetter = capsWord[i];
// score += SCORE_BOARD[currentLetter];
// }
// if (word.length >= 7) {
// score += 8;
// }
// return score;
// };

// export const highestScoreFrom = (words) => {
// let scoreArray = getScoreArray(words);
// let highScore = getHighScore(scoreArray);
// let potentialWinners = getPotentialWinners(words, scoreArray, highScore);
// let tenLetterWinner = checkForTen(potentialWinners);
// if (tenLetterWinner) {
// return {"word": tenLetterWinner, "score": highScore};
// }
// else {
// let lengthArray = getLengthArray(potentialWinners);
// let tieBreaker = getTieBreaker(potentialWinners, lengthArray);
// return {"word": tieBreaker, "score": highScore};
// }
// };

// const getScoreArray = (words) => {
// let scoreArray = []
// for (let i = 0; i < words.length; i++) {
// let currentWord = words[i];
// let currentScore = scoreWord(currentWord);
// scoreArray.push(currentScore);
// }
// return scoreArray;
// };

// const getHighScore = (scoreArray) => {
// let highScore = Math.max(...scoreArray);
// return highScore;
// };

// const getPotentialWinners = (words, scoreArray, highScore) => {
// let potentialWinners = [];
// for (let i = 0; i < scoreArray.length; i++) {
// let winner = scoreArray[i];
// let winningWord = words[i];
// if (winner === highScore) {
// potentialWinners.push(winningWord);
// }
// }
// return potentialWinners;
// };

// const checkForTen = (potentialWinners) => {
// for (let i = 0; i < potentialWinners.length; i++) {
// let currentWord = potentialWinners[i];
// if (currentWord.length === 10) {
// return currentWord;
// }
// }
// };

// const getLengthArray = (potentialWinners) => {
// let lengthArray = []
// for (let i = 0; i < potentialWinners.length; i++) {
// let currentWord = potentialWinners[i].length;
// lengthArray.push(currentWord);
// }
// return lengthArray;
// };

// const getTieBreaker = (potentialWinners, lengthArray) => {
// for (let i = 0; i < lengthArray.length; i++) {
// let currentWord = potentialWinners[i];
// let shortestLength = Math.min(...lengthArray);
// if (currentWord.length === shortestLength) {
// return currentWord;
// }
// }
// };
Loading