Conversation
There was a problem hiding this comment.
Inconsistent use of spaces and tabs between files. I recommend using a linter like rubocop.
Also, to maintain consistent indentation in your code you can use http://editorconfig.org/
| loop do | ||
| Print::score(scorekeeper.player_score, scorekeeper.computer_score) | ||
| case Print::new_match | ||
| when "r", "R" |
There was a problem hiding this comment.
could simplify by this by doing something like this:
foo = Print::new_match.downcase
case foo
when 'r'
when 'p'etc
| end | ||
|
|
||
| def evaluate(player_move) | ||
| @computer_move = generate_computer_move |
There was a problem hiding this comment.
Since you've defined an attr_acessor for this variable, the @ in unnecessary.
Here's a pretty thorough explanation on stack overflow: http://stackoverflow.com/questions/5046831/why-use-rubys-attr-accessor-attr-reader-and-attr-writer
It's a pretty simple concept once you get the hang of it!
| @player_move = player_move | ||
| @win = nil | ||
| if @player_move!=@computer_move | ||
| if (@player_move == :rock && @computer_move == :scissors) || |
There was a problem hiding this comment.
This is a pretty heavy if statement. You could extract this away into a method call, something like did_player_win? or winning_move?. Abstracting obtuse logic into nicely named method is a great way to help the next person to work on your code understand your intent!
| end | ||
|
|
||
| def generate_computer_move | ||
| prng = Random.new |
There was a problem hiding this comment.
Since this is pretty short you could combine this line and the on below it since the instance is only used 1 time.
res = Random.new.rand(3)
| def generate_computer_move | ||
| prng = Random.new | ||
| res = prng.rand(3) | ||
| if res == 0 |
There was a problem hiding this comment.
Another way this could be handled is initializing a collection
moves = [:rock, :paper, :scissors].freeze
moves[res]
This is cool since you can also make this a class constant, and reuse it/establish an interface for dealing with the "moves".
3 participants
@jaybobo