-
Notifications
You must be signed in to change notification settings - Fork 26
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
Ampers: Nora and Victoria's Scrabble assignment #19
base: master
Are you sure you want to change the base?
Conversation
…re are passing when they shouldn't. We'll see.
…le statement does
…les. Cleaned up the errors the retrofitting caused in the tests.
…ey over the PA. So we have that, if nothing else.
…f the Wave 3 things. Party.
…ss to remove superfluous comments, etc., but this is basically done.
ScrabbleWhat We're Looking For
|
Good job! Sorry that this feedback is so late! Great job with the enumerables methods you've used. It helped make really clever solutions! You all handled and tested for some tricky cases in I think the code is straightforward and lean overall. Maybe one line that got pretty long and tricky would be this one:
I think it makes sense overall, it just might've benefited from breaking it into two lines. Also, in the
I think maybe a more typical default value may be Victoria, I know you and I talked about how you would like me to look at a specific piece of code in Overall, great job. Cheers! |
It wasn't so much a specific piece of code, I think, as our general
approach to Tilebag. Specifically, I/we(?) were worried, because in our
version, we pass in the contents of the Tilebag when it's instantiated,
instead of having it generate its own contents from a constant, as most
people seem to have done. We went with passed-in contents because that
made it easier to test, but we weren't sure if that would have consequences
we didn't plan for.
…On Fri, Mar 2, 2018 at 5:12 PM, dee ***@***.***> wrote:
Good job!
Sorry that this feedback is so late!
Great job with the enumerables methods you've used. It helped make really
clever solutions!
You all handled and tested for some tricky cases in Scoring's
highest_score_from method: good job for dealing with that, a lot of
people missed it.
I think the code is straightforward and lean overall. Maybe one line that
got pretty long and tricky would be this one:
winning_indices = scores_array.each_with_index.select { |score, index| score == high_score }.map { |array| array[1]}
I think it makes sense overall, it just might've benefited from breaking
it into two lines.
Also, in the score method, you all provided the default value "x" when
using fetch:
tile_value = LETTERVALUES.fetch(letter, "x")
I think maybe a more typical default value may be false or nil.
Victoria, I know you and I talked about how you would like me to look at a
specific piece of code in TileBag and unfortunately I forgot which piece
of functionality. Do you mind reminding me which?
Overall, great job. Cheers!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#19 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AZA9Up4MtFumg_NauGhOJwRl_HM3xXnHks5tae4JgaJpZM4SSFjj>
.
|
Scrabble
Congratulations! You're submitting your assignment.
Comprehension Questions
score
method in theScoring
class a class method instead of an instance method?Enumerable
mixin? If so, where and why was it helpful?