Conversation
gem_city.rb
Outdated
| def initialize | ||
| @people = { | ||
| thieves: 5, | ||
| officers: 1 } |
There was a problem hiding this comment.
It would be simpler if these weren't stuck together in @people. Try defining them as @thieves and @officers.
|
Good point. |
gem_city.rb
Outdated
| end | ||
|
|
||
| def successful_crime_rate | ||
| if @thieves <= 0 || @officers > thieves |
There was a problem hiding this comment.
Whenever these if conditions get even a little complicated, I think they should be extracted to a private method. What do you think a good name for this condition might be? no_crime_can_occur?, plenty_of_cops?, something like that.
There was a problem hiding this comment.
I prefer no_crime_can_occur? because it captures the meaning of the method without exposing how the method makes that decision.
gem_city.rb
Outdated
| end | ||
|
|
||
| def no_crimes_can_occur? | ||
| @thieves <= 0 || @officers > @thieves |
There was a problem hiding this comment.
I think it is a little better to use the reader methods for instance variables even inside the class. In the off chance they get refactored into real methods someday, this line shouldn't need to change.
There was a problem hiding this comment.
Is there a way to do that without angering rubocop?
There was a problem hiding this comment.
It was complaining about me not understanding the intent until just now. I think that I have it now.
|
This is looking real nice. 👍 |
|
Thank you. |
2 participants
Thanks @jaybobo @mikegee