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

Adagrams Submission - Danielle & Alice #2

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

Conversation

danimetz
Copy link

@danimetz danimetz commented Aug 15, 2018

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
What are the components that make up a method? Method signature (optional parameters), method body - block of code, explicit or implicit return
What are the advantages of using git when collaboratively working on one code base? You can simultaneously work on the code, by pushing and pulling from github, and it will keep track of all the different versions and commit messages.
What kind of relationship did you and your pair have with the unit tests? It allowed us to understand our expected outputs for our methods. If there were errors, we could then isolate where our program was failing.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? .all? was used to make sure all the letters were in the hand. .max_by was used to determine what the max score was. .select was used to isolate values that had the max score. Min_by was used to determine the minimum length of the words that had max scores. Then we used .select again to isolate words & scores that had the max score but were also the shortest or 10 letters long. We also used include? to determine if the words were in the english dictionary.
What was one method you and your pair used to debug code? We ran adagram.rb and used puts statements to see what was being returned. We also used .class to determine the structure of the data being returned so we could access the data later in our code.
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? There was a good balance of give and take. We were both flexible when it came to sharing ideas and strategies and were willing to try different ways for solving each of the waves.

count = 0
while count < 10 do
index = rand(0...letter_probability.length)
if letter_probability[index] != 0

Choose a reason for hiding this comment

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

So, this works, but you end up with an array that becomes more and more likely to "bounce" as you pick letters from the pile. Instead of nulling the value at the index, why not remove just that element from the array? That way, you don't waste loops through on no-ops (no operation).

return false
else
input.each do |letter|
input.count(letter) > letters_in_hand.count(letter) ? value << false : value << true

Choose a reason for hiding this comment

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

You are doing more work than you need to! As soon as one of these checks comes up false, you can return false, right? The only case that requires you to complete the loop is if every letter matches!

input.count(letter) > letters_in_hand.count(letter) ? value << false : value << true
end
end
value.all? true ? true : false

Choose a reason for hiding this comment

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

value.all? is hiding a second loop from you!

@dHelmgren
Copy link

Adagrams

What We're Looking For

Feature Feedback
General
Answered comprehension questions Good, comprehensive answers
Both teammates contributed to the codebase +
Small commits with meaningful commit messages Commits were lean and clean!
Code Requirements
draw_letters method It works, but see my comment!
Uses appropriate data structure to store the letter distribution Conversion from a hash to an array was easy to miss, but I can understand why you opted for that!
All tests for draw_letters pass Yes indeed!
uses_available_letters? method Again, good, but check the comments!
All tests for uses_available_letters? pass Green across the board!
score_word method * Big Thumbs Up *
Uses appropriate data structure to store the letter scores So, it's not in a data structure, but your case statement is good!
All tests for score_word pass P-P-P-PASS
highest_score_from method Big thumbs up!
Appropriately handles edge cases for tie-breaking logic Good work on this. Clear flow, correct output!
All tests for highest_score_from pass Yupp!
Overall

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.

3 participants