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

Ashley_Benson_Seaturtles #116

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
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ Our first job is to build a hand of 10 letters. To do so, implement the function
- Each string should contain exactly one letter
- These represent the hand of letters that the player has drawn
- The letters should be randomly drawn from a pool of letters
- This letter pool should reflect the distribution of letters as described in the table below
- This letter pool should reflect the distribution of letters as describe.skipd in the table below

Choose a reason for hiding this comment

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

When doing global search/replace across multiple files, be sure to check for unintended side effects.

- There are only 2 available `C` letters, so `drawLetters` cannot ever return more than 2 `C`s
- Since there are 12 `E`s but only 1 `Z`, it should be 12 times as likely to draw an `E` as a `Z`
- Invoking this function should **not** change the pool of letters
Expand Down Expand Up @@ -143,8 +143,8 @@ Next, we need a way to check if an input word (a word a player submits) only use
To do so, implement the function named `usesAvailableLetters` in `src/adagrams.js`. This function should have the following properties:

- Has two parameters:
- `input`, the first parameter, describes some input word, and is a string
- `lettersInHand`, the second parameter, describes an array of drawn letters in a hand. You can expect this to be an array of ten strings, with each string representing a letter
- `input`, the first parameter, describe.skips some input word, and is a string
- `lettersInHand`, the second parameter, describe.skips an array of drawn letters in a hand. You can expect this to be an array of ten strings, with each string representing a letter
- Returns either `true` or `false`
- Returns `true` if every letter in the `input` word is available (in the right quantities) in the `lettersInHand`
- Returns `false` if not; if there is a letter in `input` that is not present in the `lettersInHand` or has too much of compared to the `lettersInHand`
Expand All @@ -158,7 +158,7 @@ Implement the function named `scoreWord` in `src/adagrams.js`. This method shoul
- Has one parameter: `word`, which is a string of characters
- Returns an integer representing the number of points
- Each letter within `word` has a point value. The number of points of each letter is summed up to represent the total score of `word`
- Each letter's point value is described in the table below
- Each letter's point value is describe.skipd in the table below
- If the length of the word is 7, 8, 9, or 10, then the word gets an additional 8 points

#### Score chart
Expand Down Expand Up @@ -194,7 +194,7 @@ Implement the function named `highestScoreFrom` in `src/adagrams.js`. This metho

This Wave has 3 parts:

1. Adjust the syntax for the tests for `highestScoreFrom` to run, instead of skip. To do this, find the `describe` block for the tests of `highestScoreFrom`, and change the syntax from `describe.skip` to `describe`
1. Adjust the syntax for the tests for `highestScoreFrom` to run, instead of skip. To do this, find the `describe.skip` block for the tests of `highestScoreFrom`, and change the syntax from `describe.skip.skip` to `describe.skip`
2. Write the pseudocode for this function, using whatever resources and references
3. Translate the pseudocode into JavaScript, using whatever resources and references

Expand Down
152 changes: 147 additions & 5 deletions src/adagrams.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,157 @@
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
}

// need to convert #'s back to int

const scoreDict = {
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,
}
const alphabet = "abcdefghijklmnopqrstuvwxyz".toUpperCase().split("");

Choose a reason for hiding this comment

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

This list of characters isn't really needed (see related comment in scoreWord implementation).






// Implement this method for wave 1
export const drawLetters = () => {
// Implement this method for wave 1
const allLetters = []
for (const [letter, frequency] of Object.entries(letterPool)) {
for (let i = 0; i < frequency; i++) {
allLetters.push(letter)
}
}
Comment on lines +68 to +73

Choose a reason for hiding this comment

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

Nice approach to build up a list of all possible letter tiles.


const hand = []

for (let i = 0; i < 10; i++) {
const thisLetter = allLetters[Math.floor(Math.random() * allLetters.length)]

Choose a reason for hiding this comment

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

Rather than getting the letter directly, then need ing to go back and look up where it was later, consider storing the calculated index

  const randomPos = Math.floor(Math.random() * allLetters.length);

and using that location to both look up the letter to push into the hand, as well as for removing it from the available letter tiles (avoiding the indexOf call).

hand.push(thisLetter)
let letterIndex = allLetters.indexOf(thisLetter)
allLetters.splice(letterIndex, 1)

Choose a reason for hiding this comment

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

Remember that removing items from an array in O(n). For an arbitrary number of tiles, this would be a O(n^2) operation (but since we're artificially restricting to 10 tiles, we can still say this is O(10n) → O(n)). Consider shuffling the array and then picking any contiguous block of 10 tiles as the hand. This additive approach would remain O(n) regardless of the number of tiles we needed to pick. In general, try to avoid subtractive approaches (at least on arrays) since that will trend towards O(n^2) overall approaches.

}
return hand

};


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

Choose a reason for hiding this comment

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

This has the benefit of both making a duplicate (so we don't destroy the original word) as well as letting us treat the input word in a case insensitive fashion (assuming the hand is built using capitals). For safety, we should probably capitalize the letters from the hand as well.


for (let letter of capsWord) {

Choose a reason for hiding this comment

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

letter could be const here. In a for/of, the variable is recreated each time through the loop (unlike a general for loop, where it persists across iterations), so we can take advantage of marking the loop control variable const.

const test = lettersInHand[letter]

Choose a reason for hiding this comment

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

This doesn't appear to be used. letterInHand is an array of letters (indexed by position) not a dictionary/object (indexed by a key), so this is likely to be undefined anyway (for/of on an array returns the values, which would be the tile letters, for example 'e', and letterInHand['e'] would be undefined).

if (lettersInHand.includes(letter)) {
let letterIndex = lettersInHand.indexOf(letter)
lettersInHand.splice(letterIndex, 1)
Comment on lines +94 to +96

Choose a reason for hiding this comment

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

includes, indexOf, and splice are all O(n) operations, and since we ware looping over the word, this trends toward O(n^2) for words of arbitrary length. We know for the adagrams project that no word can be more than 10 letters, which would let us argue that this is O(10n) → O(n), but consider using frequency maps calculated from the word and hand as a way of comparing whether the word can be made using tiles in the hand, without needing to iterate through the hand for every letter in the word.

} else {
return false
}
}
return true

};


// Implement this method for wave 3
export const scoreWord = (word) => {
// Implement this method for wave 3
const input = word.toUpperCase();
const input_array = input.split("");
const n = input.length;
let score = 0;
for (let i = 0; i < n; i++) {
let letter = input_array[i];
Comment on lines +112 to +113

Choose a reason for hiding this comment

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

Consider iterating over the letters in the (capitalized) word directly using for/of

  for (const letter of word.toUpperCase()) {
    ...

if (alphabet.includes(letter)) {

Choose a reason for hiding this comment

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

includes is a O(n) operation. To check whether the letter is in the scoreDict, we can use the in operator (similar to Python)

  if (letter in scoreDict) {
    ...

score += scoreDict[letter];
}
}
if (n > 6 && n < 11) {
score += 8;
}
return score;
};

// Implement this method for wave 4
export const highestScoreFrom = (words) => {
// Implement this method for wave 1
};

const n = words.length;
let high_score = {
word: "",
score: 0,
};

for (let i = 0; i < n; i++) {

Choose a reason for hiding this comment

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

Consider iterating over the words in the candidate list directly using for/of

  for (const word of words) {
    ...

then replace any references to words[i] with word. This has a much smaller chance of introducing a typo than repeated needing to reference words[i].

if (scoreWord(words[i]) > high_score.score) {

Choose a reason for hiding this comment

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

Calculate the word score once before performing all of these checks

  const wordScore = scoreWord(word);  // assumes for/of suggestion above
  ...

and then replace scoreWord(word) with wordScore. This has a much smaller chance of introducing a typo than repeated needing to reference scoreWord(word) (or especially scoreWord(words[i])).

high_score = {
word: words[i],
score: scoreWord(words[i]),
};
//tie
} else if (scoreWord(words[i]) === high_score.score) {

Choose a reason for hiding this comment

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

Nice tie-breaker logic.

if (high_score.word.length === 10) {
high_score = high_score;

Choose a reason for hiding this comment

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

Rather than re-assigning to itself, we could continue to move on to the next word, leaving the current high score unchanged.

} else if (words[i].length === 10) {
high_score = {
word: words[i],
score: scoreWord(words[i]),
};
} else if (words[i].length < high_score.word.length) {
high_score = {
word: words[i],
score: scoreWord(words[i]),
};
}
}
}
return high_score
};
Loading