Skip to content

Yasmin-Leaves #31

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Yasmin-Leaves #31

wants to merge 5 commits into from

Conversation

YasminM11
Copy link

JS Adagrams

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What patterns were you able to use from your Ruby knowledge to apply to JavaScript? using loops, arrays, .map, and how to google.
Did you need to use different strategies to find information online about JavaScript than you would for Ruby? When i was googling i was trying to find how to convert the syntax from ruby to JavaScript. I used W3School and MDN.
What was a challenge you were able to overcome on this assignment? To remember the syntax and reading the test errors.
What has been interesting and positive about learning a new programming language? not sure :) i think how to use a language that i know to write another language.
What is something to focus on your learnings in JavaScript in the next week? I need to practice more to get more familiar and understand the syntax and functions.

@dHelmgren
Copy link

JS Adagrams

What We're Looking For

Feature Feedback
General
Answered comprehension questions yes
Small commits with meaningful commit messages yes
Code Requirements
drawLetters method good
Uses appropriate data structure to store the letter distribution yes
All tests for drawLetters pass yes
usesAvailableLetters method yes
All tests for usesAvailableLetters pass yes
scoreWord method yes
Uses appropriate data structure to store the letter scores See comment! There is a DRYer way.
All tests for scoreWord pass yes
Optional
highestScoreFrom method
Appropriately handles edge cases for tie-breaking logic
All tests for highestScoreFrom pass
Overall Great job overall! While there is one spot where things could be improved, this code is generally clean and well-organized, and it feels like the JavaScript unit is off to a solid start. Keep up the hard work!

Comment on lines +19 to +22
let randomLetter = avaLetters[index];
// remove the randomLetter from avaLetters array
avaLetters.splice(index, 1);
handOfLetters.push(randomLetter);

Choose a reason for hiding this comment

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

splice returns the deleted elements, so you can smoosh these lines together like so:

handOfLetters.push( avaLetters.splice(index, 1) );

Comment on lines +45 to +66
const onePointLetter = ['A', 'E', 'I', 'O', 'U', 'L', 'N', 'R', 'S', 'T'];
const twoPointLetter = ['D', 'G'];
const threePointLetter = ['B', 'C', 'M', 'P'];
const fourPointLetter = ['F', 'H', 'V', 'W', 'Y'];
const fivePointLetter = ['K'];
const eightPointLetter = ['J', 'X'];
const tenPointLetter = ['Q', 'Z'];

let letters = word.toUpperCase().split('');
const scores = letters.map((letter) => {
let letterScore = 0;

if (onePointLetter.includes(letter)) {
letterScore += 1;
} else if (twoPointLetter.includes(letter)) {
letterScore += 2;
} else if (threePointLetter.includes(letter)) {
letterScore += 3;
} else if (fourPointLetter.includes(letter)) {
letterScore += 4;
} else if (fivePointLetter.includes(letter)) {
letterScore += 5;

Choose a reason for hiding this comment

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

This ends up being pretty difficult to read and check. I would suggest using a JS Object in the form of

hashStyleScores = {
'A': 1,
'B': 4
}

This format makes it easy to find and change each letter, and means that scoring a single letter is a one-line operation!

beccaelenzil added a commit to beccaelenzil/js-adagrams that referenced this pull request Jun 2, 2022
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