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 Morgan & Janice #11

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

Leaves Morgan & Janice #11

wants to merge 12 commits into from

Conversation

jaitch
Copy link

@jaitch jaitch commented Aug 15, 2019

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
What are the components that make up a method? def + method signature (name and parameters) + code block + end.
What are the advantages of using git when collaboratively working on one code base? So you can work on code simultaneously on separate machines, compare/merge changes, version control for when you're working non-simultaneously.
What kind of relationship did you and your pair have with the unit tests? We appreciate them for the process; it was educational to see the process broken down into stages.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? Why, yes! .map to convert letters into their corresponding points. It enabled us to alter the original array elegantly and without creating a hash at that point.
What was one method you and your pair used to debug code? We tried using 'pry' and also used puts statements.
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 our communications styles complemented each other; we both lean toward the 'ask' end of the ask-guess culture spectrum. We did a good job of trading roles and sharing/splitting tasks.

@beccaelenzil
Copy link

Adagrams

What We're Looking For

Feature Feedback
General
Answered comprehension questions yes
Small commits with meaningful commit messages yes
Code Requirements
draw_letters method yes
Uses appropriate data structure to store the letter distribution consider making this structure a global variable
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 onsider making this structure a global variable
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 Hey y'all! Great work on this project. Your code fulfills all of the requirements and does it in a readable and reasonable way. The code is clean, and it passes all the tests! I've made a few comments on this assignment about some suggestions on how to refactor. One this to make note of is to remove unused code and pay attention to warning message (that often alert you to unused code :).

require 'csv'

def draw_letters
all_letters_hash = {

Choose a reason for hiding this comment

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

Consider making this hash a global variable assigned outside the method so that it can be used other place.

all_letters_array << letter
end
end
letters_in_hand = all_letters_array.sample(10)

Choose a reason for hiding this comment

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

You should explicitly return letters_in_hand. Note warning when you run rake:

"/Users/becca/Documents/GitHub/c12/adagrams/lib/adagrams.rb:38: warning: assigned but unused variable - letters_in_hand"

letter_count[letter] -= 1
if letter_count[letter] < 0
return false
break

Choose a reason for hiding this comment

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

You don't need break. When you return in a function, nothing is executed after the return.


def score_word(word)
# Make new hash to store letters and point values
points = {

Choose a reason for hiding this comment

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

Similar to all_letter_hash, it would make sense to define this hash outside the method as a global variable using all caps.

if input_array.size >= 7 && input_array.size < 11
return total_points.sum + 8
end
# if total_points.empty?

Choose a reason for hiding this comment

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

Remove unused commented code when you refactor.

return total_points.sum
end

# REPLACE HIGH SCORE HASH STEPS WITH A HELPER METHOD

Choose a reason for hiding this comment

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

This would be a great use of a helper method, and a good task for a refactor.

end

# REPLACE HIGH SCORE HASH STEPS WITH A HELPER METHOD
def highest_score_from(words)

Choose a reason for hiding this comment

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

The comments in this method are helpful and are an example of a good use of comments.

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