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

Water (Richelle & Mackenzie) Adagrams #23

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

Conversation

r-spiel
Copy link

@r-spiel r-spiel commented Sep 18, 2020

Assignment Submission: Adagrams

Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.

Reflection

Feature Feedback
What are the components that make up a method?
The signature contains the parameter(s) you need to use that are the data being pulled into your method, and then there is the body of the code and you need a return statement before the end.
What are the advantages of using git when collaboratively working on one code base?
It's easier to share code and track changes.
What kind of relationship did you and your pair have with the unit tests?
We looked at them as we were going to help guide us on what each wave required.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful?
We used .max, .split, .each, .select .include?, min_by, max_by, delete_at(just for arrays)... these were helpful to iterate through a hash/array and find the data we were seeking or move an array into a hash.
What was one method you and your pair used to debug code?
We used p to print variables as we were writing our code at different points to make sure the variable was holding the data we wanted and debug any issues.
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 were both focusing on getting the output we needed before going back and refactoring. We discussed different methods to try to get to the result we needed before we started coding, and then we chose one and start with. We crushed it.

Copy link

@kaidamasaki kaidamasaki left a comment

Choose a reason for hiding this comment

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

Great job! You made good use of a number of enumerable methods and data structures in your program.

I gave you a few tips for how you can save yourself some time in the future but everything looks solid. 😃 Congratulations on your first pair project!

Adagrams

Major Learning Goals/Code Review

Criteria yes/no, and optionally any details/lines of code to reference
Correctly creates and calls methods with proper syntax (parameters, return statements, etc.) ✔️
Uses correct syntax for conditional logic and iteration ✔️
Practices git with at least 3 small commits and meaningful commit messages ✔️
Utilizes unit tests to verify code; tests can run using the command $ rake and we see test successes and/or failures ✔️
Practices pair programming; the reflection question on pair programming is answered It looks like you forgot to fill out the reflection questions. You should make sure to give those a shot so you can organize your thoughts.

Functional Requirements

Functional Requirement yes/no
For the draw_letters method, there is an appropriate data structure to store the letter distribution. (You are more likely to draw an 'E' than an 'X'.) ✔️
Utilizes unit tests to verify code; all tests for draw_letters and uses_available_letters? pass ✔️
Utilizes unit tests to verify code; all tests for score_word pass ✔️
Utilizes unit tests to verify code; all tests for highest_score_from pass ✔️

Overall Feedback

Overall Feedback Criteria yes/no
Green (Meets/Exceeds Standards) 4+ in Code Review && 3+ in Functional Requirements ✔️
Yellow (Approaches Standards) 3+ in Code Review && 2+ in Functional Requirements
Red (Not at Standard) 0-2 in Code Review or 0,1 in Functional Reqs, or assignment is breaking/doesn’t run with less than 5 minutes of debugging

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality Yes?
Perfect Indentation
Elegant/Clever
Descriptive/Readable
Concise
Logical/Organized

#Wave 2
def uses_available_letters?(input, letters_in_hand)
valid_word = nil
input_as_array = input.upcase.split("")

Choose a reason for hiding this comment

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

You can make this a little clearer with chars:

Suggested change
input_as_array = input.upcase.split("")
input_as_array = input.upcase.chars

def uses_available_letters?(input, letters_in_hand)
valid_word = nil
input_as_array = input.upcase.split("")
hand = letters_in_hand.dup

Choose a reason for hiding this comment

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

Great job using dup here!

It's always good to make copies of your input if you are modifying it* and is really easy to forget!

*Unless the point of the method is to modify the input.

Comment on lines +24 to +42
letters_boolean = []

input_as_array.each do |letter|
if hand.include?(letter)
index = hand.index(letter)
hand.delete_at(index)
letters_boolean << true
else
letters_boolean << false
end
end

if letters_boolean.include?(false)
valid_word = false
else
valid_word = true
end

return valid_word

Choose a reason for hiding this comment

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

You can simplify this by returning early:

Suggested change
letters_boolean = []
input_as_array.each do |letter|
if hand.include?(letter)
index = hand.index(letter)
hand.delete_at(index)
letters_boolean << true
else
letters_boolean << false
end
end
if letters_boolean.include?(false)
valid_word = false
else
valid_word = true
end
return valid_word
input_as_array.each do |letter|
if hand.include?(letter)
index = hand.index(letter)
hand.delete_at(index)
else
return false
end
end
return true

This works because as soon as you find a missing letter you know that the word is invalid.

This is a really useful pattern for solving these kinds of problems in a small amount of code.

}
end

highest_scoring_word = array_of_words.max_by{ |word| word[:score].to_i}

Choose a reason for hiding this comment

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

to_i is redundant here since score_word already returns an Integer.

Suggested change
highest_scoring_word = array_of_words.max_by{ |word| word[:score].to_i}
highest_scoring_word = array_of_words.max_by{ |word| word[:score] }

Comment on lines +90 to +91
ties = []
ties = array_of_words.select {|word| word[:score] == highest_scoring_word[:score]} #an array of hashes of the ties

Choose a reason for hiding this comment

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

You left in an extra initialization of ties but great use of select!

Suggested change
ties = []
ties = array_of_words.select {|word| word[:score] == highest_scoring_word[:score]} #an array of hashes of the ties
ties = array_of_words.select {|word| word[:score] == highest_scoring_word[:score]} #an array of hashes of the ties

Comment on lines +93 to +110
winner = []

ties.each do |word_hash|
if word_hash[:length] == 10
winner << word_hash
end
end

if winner.empty?
winner << ties.min_by{ |word_hash| word_hash[:length].to_i }
end

winning_word = {}

winning_word[:word] = winner[0][:word]
winning_word[:score] = winner[0][:score]

return winning_word

Choose a reason for hiding this comment

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

You can simplify this using find (also, generally you can leave types off of variable names):

Suggested change
winner = []
ties.each do |word_hash|
if word_hash[:length] == 10
winner << word_hash
end
end
if winner.empty?
winner << ties.min_by{ |word_hash| word_hash[:length].to_i }
end
winning_word = {}
winning_word[:word] = winner[0][:word]
winning_word[:score] = winner[0][:score]
return winning_word
return ties.find { |word| word[:length] == 10 } || ties.min_by { |word| word[:length] }

The || here works because find returns nil if nothing is found and nil is falsey.

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