-
Notifications
You must be signed in to change notification settings - Fork 33
Branches - Brianna + Monick #9
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
base: master
Are you sure you want to change the base?
Conversation
AdagramsWhat We're Looking For
Well done on this project, Brianna and Monick! You all have some great and solid logic, with readable code style. I can tell that you all wrote your solution and found interesting and awesome opportunities to use Enumerable methods. I have a few comments on your code to help point out some places where I think things could be cleaned up if you had more time on this project. I could also see a lot opportunities of refactoring the Otherwise, great work on this project. |
end | ||
|
||
return hand | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on this method! This is a good strategy. Actually, although you're right that sometimes you need to make a copy of variables, in this case, you don't need copy_of_letters
. You can replace copy_of_letters
here with letters
and the tests still pass! Let me know if you have questions on that!
letters_array.each do |letter| | ||
score = score_chart.find {|points, letters| | ||
letters.include?(letter) | ||
}.first |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice use of find
!
end | ||
|
||
winning_word = nil | ||
sorted_words = winning_words.sort_by(&:length) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
verdict = false | ||
end | ||
return verdict | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on this optional wave!
verdict = false | ||
end | ||
return verdict | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's an idea for a refactoring if you had more time on this project: Is there a way to take out the else ... verdict = false
? Hint: Could changing the initial value of verdict
in verdict = ""
to something else help?
verdict = false | ||
end | ||
return verdict | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, do you have any tests for this? ;)
Adagrams
Congratulations! You're submitting your assignment.
Comprehension Questions
Enumerable
mixin? If so, where and why was it helpful?