Skip to content

C22 Phoenix - Tatyana Venanzi#37

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

C22 Phoenix - Tatyana Venanzi#37
taty1202 wants to merge 3 commits into
Ada-C22:mainfrom
taty1202:main

Conversation

@taty1202

Copy link
Copy Markdown

No description provided.

@kelsey-steven-ada kelsey-steven-ada left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice work translating Adagrams to Javascript! Let me know if you have questions on any feedback =]

Comment thread src/adagrams.js
}
}
let hand = [];
for (let i = 0; i < 10; i++) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This loop declares a variable i, but we never use that variable inside the loop. A while loop that runs until the hand array has 10 elements might be a better fit.

Comment thread src/adagrams.js
Comment on lines +63 to +70
for (const letter of upperCaseInput) {
if (copyLettersInHand.includes(letter)) {
const index = copyLettersInHand.indexOf(letter);
copyLettersInHand.splice(index,1);
} else {
return false;
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This implementation has an O(n^2) time complexity, how could we use a frequency map to bring the time complexity down to O(n)?

Comment thread src/adagrams.js

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice use of the spread operator to copy lettersInHand!

Comment thread src/adagrams.js
for (const letter of word) {
totalScore += LETTER_SCORES[letter] || 0;
}
return totalScore += word.length >= 7 ? 8 : 0; //ternary expression

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Love a good ternary expression ^_^

I would recommend removing the comment at the end or reframing it to describe the purpose of this ternary expression.

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