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

Trang & Cassy - Adagram #7

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

Trang & Cassy - Adagram #7

wants to merge 19 commits into from

Conversation

tfrego
Copy link

@tfrego tfrego commented Aug 17, 2018

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
What are the components that make up a method? Method signature which includes method name and parameters and the method definition block.
What are the advantages of using git when collaboratively working on one code base? Collaborators can easily push and pull changes from one source and easily resolves merge conflicts.
What kind of relationship did you and your pair have with the unit tests? We ran the tests often to check our code and made sure previous tests were still passing.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? Yes, 1) we used .count to count letters in hand and letters in word, 2) we used .all? to verify letter quantities were valid, 3) we used .map to create an array of hashes for word scores, 4) we used .max_by to find the top score 5) we used .min_by to find the word with the least letters, 6) we used .select to select words that tied for the top score and to select words with a length of 10, 7) we used .reject to remove a key-value pair, 8) we used .any to see if a word is in the dictionary
What was one method you and your pair used to debug code? We used awesome_print to check data structures
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? 1) We decided when giving feedback by the navigator, that it would be best to let the driver finish their block of code before providing feedback. That way the driver can finish their train of thought. 2) We decided to do more planning and white-boarding before diving into coding.

@dHelmgren
Copy link

Adagrams

What We're Looking For

Feature Feedback
General
Answered comprehension questions Very thorough! Thank you!
Both teammates contributed to the codebase Just an FYI, I believe that you both worked on code, it's just that you can't tell from github.
Small commits with meaningful commit messages Yeah! You'd be able to roll back just about any bug!
Code Requirements
draw_letters method Great work! Clear and consise
Uses appropriate data structure to store the letter distribution Yeah! That array will do the trick
All tests for draw_letters pass Paaasssss
uses_available_letters? method Good stuff, though see comments!
All tests for uses_available_letters? pass PASS
score_word method This is nice, very easy to read
Uses appropriate data structure to store the letter scores I LOVE A GOOD CASE STATEMENT!
All tests for score_word pass P-P-P-Pass!
highest_score_from method Nice stuff!
Appropriately handles edge cases for tie-breaking logic Sure does!
All tests for highest_score_from pass Passerino!
Overall Nice work across the board. Be aware that enumerators hide loops, and that running through a loop after a loop is not the most efficient. Also, when your logic becomes really compact, it helps to have verbose comments to clarify.

# Validating quantitiy
input.chars.each do |letter|
if input.count(letter) > letters_in_hand.count(letter)
letter_results << 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 know one is false, do you need to see the rest of the letters?

end
end
# Returns true if input quantity is valid
return letter_results.all?(true)

Choose a reason for hiding this comment

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

This is hiding a second loop from you!

if length_of_10.length > 0
# Returning first word/score in the length_of_10 array
return { :word => length_of_10[0][:word], :score => length_of_10[0][:score]}
else

Choose a reason for hiding this comment

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

The logic after here is kind of hairy, could have benefited from a longer comment.

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