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

Leaves - Yitgop & Bri #19

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

Leaves - Yitgop & Bri #19

wants to merge 7 commits into from

Conversation

rinostar
Copy link

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
What are the components that make up a method? signature, parameter, invocation/return
What are the advantages of using git when collaboratively working on one code base? real time updates from multiple team members, version control tracking, comments/team communication
What kind of relationship did you and your pair have with the unit tests? Helpful in the final phase - we could have used it sooner
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? We used: map (18, 67, 80 line), max (line 83), min (line 105), select, sample, uniq (line 80), zip (line 83). The methods each condensed the code and helped improve clarity.
What was one method you and your pair used to debug code? We used pry and the tests
What are two discussion points that you and your pair discussed when giving/receiving feedback from each other that you would be willing to share? We discussed how helpful it was for us both to break down the steps, draw out our approach and take the time to explain it to each other. We also were both willing to be flexible, even when it meant to start the approach over from scratch. Be willing to take breaks and communicate your needs.

@dHelmgren
Copy link

Adagrams

What We're Looking For

Feature Feedback
General
Answered comprehension questions good
Small commits with meaningful commit messages feel free to leave of the parts about waves in the future! In industry, there are no waves!
Code Requirements
draw_letters method
Uses appropriate data structure to store the letter distribution yes
All tests for draw_letters pass yes
uses_available_letters? method
All tests for uses_available_letters? pass yes
score_word method
Uses appropriate data structure to store the letter scores yes
All tests for score_word pass yes
highest_score_from method
Appropriately handles edge cases for tie-breaking logic yes
All tests for highest_score_from pass yes
Overall Great job on this overall! Check my comments for some minor improvements!

@@ -143,7 +147,8 @@
# verify both have a score of 10
expect(score_word(words.first)).must_equal 18
expect(score_word(words.last)).must_equal 18

require "pry"
binding.pry

Choose a reason for hiding this comment

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

please please please! Take all the binding.pry's out of your code before submission!

@@ -0,0 +1,99 @@
# This is our data structure
pool = ["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

Choose a reason for hiding this comment

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

this is the first time I've seen this done in this way, and it's ingenious!

# WAVE#1
def draw_letters
letters_in_hand = POOL
return letters_in_hand.sample(10)

Choose a reason for hiding this comment

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

this is ostensibly the same as doing:

  return POOL.sample(10)

hand_hash[letter] -= 1
letter_check << "True"
elsif hand_hash[letter] == 0
letter_check << "False"

Choose a reason for hiding this comment

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

As soon as you fail to find one of the letters, you can bail! just return false right the heck here!

score += score_chart[letter]
end

if word_array.length == 7 || word_array.length == 8 || word_array.length == 9 || word_array.length == 10

Choose a reason for hiding this comment

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

All these || in here makes it hard to read. You can do this with two comparisons, what are they?

if word_array.length >= 7 && word_array.length <= 10

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