Skip to content

C22 Sphinx Somy C#28

Open
PickledData wants to merge 3 commits into
Ada-C22:mainfrom
PickledData:main
Open

C22 Sphinx Somy C#28
PickledData wants to merge 3 commits into
Ada-C22:mainfrom
PickledData:main

Conversation

@PickledData

Copy link
Copy Markdown

No description provided.

@mikellewade mikellewade left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice work! Please let me know if there are any questions about the comments made.

Comment thread src/adagrams.js
Comment on lines +8 to +16
const letterValue = new Map([
[['A', 'E', 'I', 'O', 'U', 'L', 'N', 'R', 'S', 'T'], 1],
[['D', 'G'], 2],
[['B', 'C', 'M', 'P'], 3],
[['F', 'H', 'V', 'W', 'Y'], 4],
[['K'], 5],
[['J', 'X'], 8],
[['Q', 'Z'], 10]
]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice to see you using different constructors and getting familiar with them!

Comment thread src/adagrams.js
// Implement this method for wave 1

let lettersList = [];
for (const [letter, count] of Object.entries(letterPool)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice work finding the javascript version of .items!

Comment thread src/adagrams.js
}

let hand = [];
let temporaryLettersList = [...lettersList];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice, love to see the spread syntax being used!

Comment thread src/adagrams.js

for (let i = 0; i < 10; i++) {
const randomIndex = Math.floor(Math.random() * temporaryLettersList.length);
const randomLetter = temporaryLettersList.splice(randomIndex, 1)[0];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These are letter counts will the same size but what if they weren't? What if the input size varied? What would be the time complexity of your code? What CS fundamentals data structure did we learn about that is handy for keeping track of the number of occurrences?

Comment thread src/adagrams.js
}

const index = lettersInHand.indexOf(letter);
lettersInHand.splice(index, 1);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You are currently mutating a mutable object, if the caller of the function is not expecting this it could lead to some difficult to debug errors.

Comment thread src/adagrams.js
for (let i = 0; i < word.length; i++) {
let letter = word[i].toUpperCase();

for (let [letters, score] of letterValue) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Since you used a Map object you get access to this nice concise/readable syntax!

Comment thread src/adagrams.js
Comment on lines +55 to +73
if (word.length === 0){
return 0;
}

let totalScore = 0
for (let i = 0; i < word.length; i++) {
let letter = word[i].toUpperCase();

for (let [letters, score] of letterValue) {
if (letters.includes(letter)) {
totalScore += score;
break;
}
}
}
if (word.length > 6){
totalScore += 8
}
return totalScore

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great function all around!

Comment thread src/adagrams.js
Comment on lines +85 to +105
for (let word of words) {
let currentScore = scoreWord(word);

if (currentScore > winner.score) {
winner = { word, score: currentScore };
}

else if (currentScore === winner.score) {
if (word.length === 10 && winner.word.length !== 10) {
winner = { word, score: currentScore };
}

else if (
word.length !== 10 &&
winner.word.length !== 10 &&
word.length < winner.word.length
) {
winner = { word, score: currentScore };
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your nested statements look good, easy to follow!

Comment thread test/adagrams.test.js
Comment on lines +123 to +125
expectScores({
"": 0
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⭐️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants