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

Branches - Julia & Eve #14

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

Branches - Julia & Eve #14

wants to merge 13 commits into from

Conversation

tofuandeve
Copy link

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
What are the components that make up a method? A method signature includes method name and parameters, a block of code that will be executed. A method can return a value (with return keyword) or nil (if method only prints output). In order for a method to execute, one must call/ invoke with arguments if parameters are set.
What are the advantages of using git when collaboratively working on one code base? Git makes it easier for remotely updating/ maintaining code by multiple people.
What kind of relationship did you and your pair have with the unit tests? We ran the tests before implementing code to see the tests fail. And as we wrote code, we ran tests again to make sure all tests pass. When we were done refactoring code, we ran tests again to make sure the tests still pass.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? We used .map, .select, .max_by and .min_by on Wave 4. Because they allowed us to easily pull values out of our data and convert data structures.
What was one method you and your pair used to debug code? We used pry to debug variables with unexpected values on Wave 4. Reading stack trace to find where the bugs were originating.
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 figured out how to be more communicative and effective throughout the project as we figured out our workflow and communication styles.

@tildeee
Copy link

tildeee commented Aug 23, 2019

Adagrams

What We're Looking For

Feature Feedback
General
Answered comprehension questions x
Small commits with meaningful commit messages x, I would prefer commit messages that are less "Implemented wave 2" and more "implements the score_word method" or "refactors the score table data structure" etc
Code Requirements
draw_letters method x
Uses appropriate data structure to store the letter distribution x
All tests for draw_letters pass x
uses_available_letters? method x
All tests for uses_available_letters? pass x
score_word method x
Uses appropriate data structure to store the letter scores x
All tests for score_word pass x
highest_score_from method x
Appropriately handles edge cases for tie-breaking logic x
All tests for highest_score_from pass x
Overall

Eve and Julia!! You two have a fantastic solution! I see a lot of thought put into your submission; you all achieved not only a good, logical, readable solution, but it has obviously gone through refactoring to make it as clever and elegant as possible within this time.

Some of the only things I could think of for improvement on this submission would be to add in the unit tests for the optional wave 5, and practice using more descriptive commit messages.

Otherwise, please keep up the clarity in logic and code, and maintain the refactoring when you can. Great work on this submission!

drawn_letters = []
10.times do
letters.shuffle!
drawn_letters << letters.pop
Copy link

Choose a reason for hiding this comment

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

Nice! Using .pop here is concise and a good call

characters.each do |char|
index = current_letters_in_hand.find_index(char)
# if index number is truthy, delete element at index
if index
Copy link

Choose a reason for hiding this comment

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

Nice logic here that relies on truthiness and understanding what find_index does!

current_letters_in_hand.delete_at(index)
# if index number is falsey, return false
else
return false
Copy link

Choose a reason for hiding this comment

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

Nice logic; putting a return false here will break the method out as soon as it needs to

winner = winner.empty? ? high_scores.min_by { |element| element.length } : winner[0]

return { word: winner, score: best_result[:score] }
end
Copy link

Choose a reason for hiding this comment

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

I'm obsessed with y'alls implementation of this method! It's so clever and so concise! It looks so good! The only thought for improvement is maybe using a different variable name besides obj. Otherwise, I love how it reads!


# WAVE 5

def is_in_english_dict?(input)
Copy link

Choose a reason for hiding this comment

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

Great work on the optional! :D ...Tests? ;)

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