Skip to content

Kunzite - Aisha M. #141

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

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

Kunzite - Aisha M. #141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
76b021c
waves 1 and 2 tests pass
aimo22 Jun 6, 2023
17018ba
wave 3 completed
aimo22 Jun 7, 2023
c89fcfa
wave 4 completed
aimo22 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
222 changes: 218 additions & 4 deletions src/adagrams.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,229 @@
export const drawLetters = () => {
// Implement this method for wave 1
const letterPool = [
"A",
"A",
"A",
"A",
"A",
"A",
"A",
"A",
"A",
"B",
"B",
"C",
"C",
"D",
"D",
"D",
"D",
"E",
"E",
"E",
"E",
"E",
"E",
"E",
"E",
"E",
"E",
"E",
"E",
"F",
"F",
"G",
"G",
"G",
"H",
"H",
"I",
"I",
"I",
"I",
"I",
"I",
"I",
"I",
"I",
"J",
"K",
"L",
"L",
"L",
"L",
"M",
"M",
"N",
"N",
"N",
"N",
"N",
"N",
"O",
"O",
"O",
"O",
"O",
"O",
"O",
"O",
"P",
"P",
"Q",
"R",
"R",
"R",
"R",
"R",
"R",
"S",
"S",
"S",
"S",
"T",
"T",
"T",
"T",
"T",
"T",
"U",
"U",
"U",
"U",
"V",
"V",
"W",
"W",
"X",
"Y",
"Y",
"Z",
];

const shuffledLetterPool = letterPool.sort(() => Math.random() - 0.5);
Copy link

Choose a reason for hiding this comment

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

This is a very interesting means of generating a random sorting of the letter pool! I think there might be a more straightforward way of accomplishing it or at least this might be the sort of code that would benefit from having a comment explaining how it works.


let hand = shuffledLetterPool.slice(0, 10);
Copy link

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.


return hand;
};

export const usesAvailableLetters = (input, lettersInHand) => {
// Implement this method for wave 2
let loop_count = 0;
for (let letter of input) {
Copy link

Choose a reason for hiding this comment

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

Since the variables on lines 111-112 aren't reassigned, you should use const over let.

for (let handLetter of lettersInHand) {
if (letter.toUpperCase() == handLetter.toUpperCase()) {
loop_count++;
const index = lettersInHand.indexOf(handLetter);
if (index > -1) {
lettersInHand.splice(index, 1);
}
}
}
}
if (loop_count === input.length) {
return true;
} else {
return false;
}
Copy link

Choose a reason for hiding this comment

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

The approach taken here definitely works! When writing code, it's always important to consider its complexity. It's a good idea to ask oneself, is there a simpler approach? For example, if you make a shallow copy of the lettersInHand argument, and perform the check to see whether a letter from the input argument is in the lettersInHand and remove it if so, otherwise if you find a letter that's not in the hand, immediately return false, you could simplify the code to something like:

const lettersInHandCopy = [...lettersInHand];
for (const letter in input.toUpperCase()) {
  if (lettersInHandCopy.includes(letter)) {
      const index = lettersInHandCopy.indexOf(letter);
      lettersInHandCopy.splice(index, 1);
  } else {
    // we've found a letter not in the hand, so we don't need to check anything else!
    return false;
  }
  // if we haven't returned yet, then all the letters are in the hand, so we can return true!
  return true;
}

};

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

if (word.length > 6 && word.length < 11) {
score += 8;
}

for (let letterCaseSensitive of word) {
Copy link

Choose a reason for hiding this comment

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

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

const letter = letterCaseSensitive.toUpperCase();
if (
letter === "A" ||
Copy link

Choose a reason for hiding this comment

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

Good use of === to ensure strict comparsion!

letter === "E" ||
letter === "I" ||
letter === "O" ||
letter === "U" ||
letter === "L" ||
letter === "N" ||
letter === "R" ||
letter === "S" ||
letter === "T"
) {
score += 1;
} else if (letter === "D" || letter === "G") {
score += 2;
} else if (
letter === "B" ||
letter === "C" ||
letter === "M" ||
letter === "P"
) {
score += 3;
} else if (
letter === "F" ||
letter === "H" ||
letter === "V" ||
letter === "W" ||
letter === "Y"
) {
score += 4;
} else if (letter === "K") {
score += 5;
} else if (letter === "J" || letter === "X") {
score += 8;
} else if (letter === "Q" || letter === "Z") {
score += 10;
}
}
Copy link

Choose a reason for hiding this comment

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

This if/else approach to scoring the word works perfectly well! However, it's a good idea to consider ways of simplifying code and control flow structures. For example, here, you could use a data structure that maps letters to their score values and iterate over the word to look up the score, adding it to the total, e.g.

const letterValue = {
  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 score = 0;
for (const letter of input) {
  score += letterValue[letter];
}


return score;
};

console.log(scoreWord("A"));
Copy link

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.


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

Choose a reason for hiding this comment

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

This variable should be a const also. Adding key/value pairs to an object is not the same as reassigning it. This is also true for adding/removing elements from arrays.


for (let word of words) {
Copy link

Choose a reason for hiding this comment

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

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

const key = word;
const value = scoreWord(word);

wordsAndScores[key] = value;
}

let potentialWinners = [];
const vals = Object.values(wordsAndScores);
const max = Math.max(...vals);
for (let word in wordsAndScores) {
if (wordsAndScores[word] === max) {
potentialWinners.push(word);
}
}

let tenLetteredWinners = [];
let shortestWinners = [];
if (potentialWinners.length === 1) {
return { word: potentialWinners[0], score: scoreWord(potentialWinners[0]) };
} else {
for (let word of potentialWinners) {
if (word.length === 10) {
tenLetteredWinners.push(word);
} else {
var min = Math.min(...words.map((o) => o.length));
if (word.length === min) {
shortestWinners.push(word);
}
}
}
}
if (tenLetteredWinners.length === 1 || tenLetteredWinners.length > 1) {
return {
word: tenLetteredWinners[0],
score: scoreWord(tenLetteredWinners[0]),
};
} else if (tenLetteredWinners.length === 0 && shortestWinners.length === 1) {
return {
word: shortestWinners[0],
score: scoreWord(shortestWinners[0])
};
}
};
8 changes: 5 additions & 3 deletions test/adagrams.test.js
View file
Open in desktop
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
Copy link

Choose a reason for hiding this comment

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

This passes the test, but an empty input would actually be '''. ' ' in fact represents a space.

});
});

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);
Copy link

Choose a reason for hiding this comment

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

Nice job here!

});

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