-
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
Wenjie & Kat - Octos - Scrabble #5
base: master
Are you sure you want to change the base?
Conversation
…rtner Merge branch 'master' of https://github.com/kseastman/Scrabble
…t 7 letters or anything like that.
Add test parts for the method as well. Tested. Working as expected
…ow fails, refactoring code to pass.
- create constructor in TileBad class - create two methods: create_tiles and draw_tiles - create instance variable tiles_remaining
- Change all tiles to array of symbols
-Add a few in class Player -All tests for class Player passed
This test did not pass, need modify code
ScrabbleWhat We're Looking For
|
if !(letter =~ /[A-Z]/) | ||
return nil | ||
end | ||
score += letter_score(letter) |
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.
Good use of a helper method.
end | ||
|
||
if max_keys.length == 1 | ||
return word_score.key(word_score.values.max) |
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.
This is a little awkward. since there is only one max word score, you could return, max_keys.first
|
||
letter_quantity.each do |letter, quantity| | ||
quantity.times do | ||
tiles << Scrabble::Tile.new(letter) |
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.
Good choice putting the tiles into an array like this
|
||
end | ||
|
||
class Tile |
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.
Think about POODR, don't write an additional class unless you need to. This class really isn't doing anything more than a 1-letter string.
|
||
plays << word | ||
score = Scrabble::Scoring.score(word) | ||
self.total_score += score |
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.
This totally works, but you might consider making total_score
a calculated field (a method) instead of an instance variable.
|
||
# Assert | ||
result.length.must_equal 7 | ||
result.wont_equal [:A, :A, :A, :A, :A, :A, :A] |
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.
This test is unlikely, but it's not impossible.
result.must_equal 93 | ||
end | ||
|
||
it 'returns tiles remaining if less than number of tiles requested' do |
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.
Good that you're testing this, I would also assert that result
would be equal to 98.
|
||
# Assert | ||
result.must_equal 60 | ||
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.
You should also have a Player
test for drawing tiles.
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?