Weekend challenge - #2127
Conversation
| # Local cache of Rubocop remote config | ||
| .rubocop-* | ||
|
|
||
| capybara-*.html |
There was a problem hiding this comment.
I love this, you've saved yourself some work here!
| erb(:game) | ||
| end | ||
|
|
||
| get '/player_2' do |
There was a problem hiding this comment.
Could this be broken up? It feels like a lot of logic in the controller here.
Maybe each condition body (between the if/else and end) could be its own method?
There was a problem hiding this comment.
I agree; this looks like sound logic, but I would suggest separating the two conditions within the IF statement and considering separating the redirect and view statements into separate routes.
|
|
||
| describe '#match' do | ||
| it 'declares Player 1 wins' do | ||
| expect(player1).to receive(:choice).and_return(:rock).at_most(3).times |
| # Local cache of Rubocop remote config | ||
| .rubocop-* | ||
|
|
||
| capybara-*.html |
|
|
||
| require_relative "./app" | ||
|
|
||
| run RPS No newline at end of file |
There was a problem hiding this comment.
Minor issue: Add a code syntax checker to your text editor to check syntax issues
| erb(:game) | ||
| end | ||
|
|
||
| get '/player_2' do |
There was a problem hiding this comment.
I agree; this looks like sound logic, but I would suggest separating the two conditions within the IF statement and considering separating the redirect and view statements into separate routes.
|
|
||
| class Player | ||
| attr_reader :name | ||
| attr_accessor :choice |
| end | ||
|
|
||
| def throw | ||
| @choice = [:rock, :scissors, :paper].sample |
There was a problem hiding this comment.
Point of consideration: Your implementation works, which is excellent and the most important thing; however, you are not using the symbol object here. I would suggest sticking to string objects within an array for this challenge.
| def player_2 | ||
| @players[1] | ||
| end | ||
|
|
There was a problem hiding this comment.
winner, player_1 and player_2 can be refactored into attr objects
| @other_player = @players[1] | ||
|
|
||
| @declaration = "" | ||
| @players_num = players_num |
There was a problem hiding this comment.
I think this construction could be refactored and simplified.
| end | ||
|
|
||
| def declaration | ||
| @declaration |
There was a problem hiding this comment.
not necessary as you have attr'd it above
| if @winner == nil | ||
| @declaration = "It is a tie!" | ||
| elsif players_num == 2 | ||
| @declaration = "#{@winner.name} wins!" |
There was a problem hiding this comment.
I don't think the declaration variable is needed, this can be inputted directly into the view
| <p>Before we start...</p> | ||
| <form action="/game" method="post"> | ||
| <p>Please choose number of players:</p> | ||
| <p><input type="radio" id="1" name="players_num" value="1"> |
3 participants
Completed the basic and multi player functions.