Skip to content

Leaves - Yitgop #28

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 4 commits into
base: master
Choose a base branch
from
Open

Leaves - Yitgop #28

wants to merge 4 commits into from

Conversation

rinostar
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? the concept of organizing data in different structures still apply
Did you need to use different strategies to find information online about JavaScript than you would for Ruby? Since we learned Ruby, now I can search for similar function in JavaScript accordingly
What was a challenge you were able to overcome on this assignment? I got more used to the different syntax and build-in functions in JavaScript
What has been interesting and positive about learning a new programming language? It is easier and much faster this time!
What is something to focus on your learnings in JavaScript in the next week? Keep practicing the syntax and functions, and gain a better understanding of the differences and similarities between JavaScript and Ruby

@beccaelenzil
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
Uses appropriate data structure to store the letter distribution see comment
All tests for drawLetters pass yes
usesAvailableLetters method
All tests for usesAvailableLetters pass yes
scoreWord method
Uses appropriate data structure to store the letter scores yes
All tests for scoreWord pass yes
Optional
highestScoreFrom method N/A
Appropriately handles edge cases for tie-breaking logic
All tests for highestScoreFrom pass
Overall Great work on this project! In this project you’ve taken some interesting logic and worked it into JavaScript syntax. I’m adding a few comments for you to review, but overall you’ve done a great job of practicing syntax.

@@ -1,8 +1,125 @@
// 1. Set up array data structure:
const pool = Array(9).fill('A').concat(

Choose a reason for hiding this comment

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

This data structure works, but it's not very DRY and would be tricky to change. For example, suppose I said you had the wrong number of "I"s - you would have to do a lot of counting to solve the problem.

Instead you might store a hash of letter frequencies like this

letterQuantities: {
  A: 9, B: 2, ...
}

and use it to build a pool of letters dynamically.

// Implement this method for wave 1
let i = 0;
const lettersInHand = new Array(10);
while (i < 10) {

Choose a reason for hiding this comment

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

There's a small bug here. Consider how you could make sure that you don't choose the same letter 10 times (however unlikely)

}

switch (final_word.length) {
case 7:

Choose a reason for hiding this comment

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

Consider how you could refactor to DRY up this code.

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