-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Weekend challenge #2127
base: main
Are you sure you want to change the base?
Weekend challenge #2127
Changes from 10 commits
5b711d0
d701daf
24ca71c
9319ae1
48ed27a
712e559
fec99df
e67f3f6
523ce89
16831d7
b3dcf91
7e83e9e
ddaf451
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,3 +3,5 @@ | |
|
||
# Local cache of Rubocop remote config | ||
.rubocop-* | ||
|
||
capybara-*.html | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
# app.rb | ||
|
||
require 'sinatra' | ||
require 'sinatra/base' | ||
require 'sinatra/reloader' # if development? | ||
require './lib/game' | ||
require './lib/player' | ||
require './lib/multi' | ||
|
||
|
||
class RPS < Sinatra::Base | ||
configure :development do | ||
register Sinatra::Reloader | ||
end | ||
|
||
get '/' do | ||
erb(:index) | ||
end | ||
|
||
post '/game' do | ||
multi = Multi.new(params[:player_1_name], params[:player_2_name], params[:players_num].to_i) | ||
$game = multi.game_creation | ||
@game = $game | ||
erb(:game) | ||
end | ||
|
||
get '/player_2' do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
if $game.players_num == 1 | ||
$game.player_1.choice = params[:throw].to_sym | ||
$game.player_2.throw | ||
redirect '/result' | ||
elsif $game.turn == 1 | ||
$game.player_1.choice = params[:throw].to_sym | ||
$game.switch_turn | ||
@game = $game | ||
erb(:game) | ||
elsif $game.turn == 2 | ||
$game.player_2.choice = params[:throw].to_sym | ||
redirect '/result' | ||
end | ||
end | ||
|
||
get '/result' do | ||
$game.match | ||
@game = $game | ||
erb(:match) | ||
end | ||
|
||
run! if app_file ==$0 | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
# ./config.ru | ||
|
||
require_relative "./app" | ||
|
||
run RPS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor issue: Add a code syntax checker to your text editor to check syntax issues |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
# lib/game.rb | ||
|
||
require './lib/player' | ||
|
||
class Game | ||
attr_reader :players, :declaration, :players_num, :current_player, :other_player, :turn | ||
|
||
def initialize(player1, player2, players_num) | ||
@winmap = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this use of cases to simplify the winning condition |
||
:rock => :scissors, | ||
:scissors => :paper, | ||
:paper => :rock | ||
} | ||
@players = [player1, player2] | ||
@winner = nil | ||
|
||
@turn = 1 | ||
@current_player = @players[0] | ||
@other_player = @players[1] | ||
|
||
@declaration = "" | ||
@players_num = players_num | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this construction could be refactored and simplified. |
||
end | ||
|
||
def declaration | ||
@declaration | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not necessary as you have attr'd it above |
||
end | ||
|
||
def switch_turn | ||
@current_player = player_2 | ||
@other_player = player_1 | ||
@turn = 2 | ||
end | ||
|
||
def winner | ||
@winner | ||
end | ||
|
||
def player_1 | ||
@players[0] | ||
end | ||
|
||
def player_2 | ||
@players[1] | ||
end | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. winner, player_1 and player_2 can be refactored into |
||
def match | ||
if player_1.choice == player_2.choice | ||
@winner = nil | ||
elsif @winmap[player_1.choice] == player_2.choice | ||
@winner = player_1 | ||
else | ||
@winner = player_2 | ||
end | ||
|
||
if @winner == nil | ||
@declaration = "It is a tie!" | ||
elsif players_num == 2 | ||
@declaration = "#{@winner.name} wins!" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think the declaration variable is needed, this can be inputted directly into the view |
||
else | ||
if @winner == player_1 | ||
@declaration = "You Win!" | ||
else | ||
@declaration = "You Lose!" | ||
end | ||
end | ||
end | ||
|
||
# private | ||
|
||
# def pick_winner | ||
# if player_1.choice == player_2.choice | ||
# @winner = nil | ||
# elsif @winmap[player_1.choice] == player_2.choice | ||
# @winner = player_1 | ||
# p "there #{@winner}" | ||
# else | ||
# @winner = player_2 | ||
# p "here" | ||
# end | ||
# end | ||
|
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
# lib/multi.rb | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor point: I suggest renaming this file to be more self explanatory |
||
|
||
require './lib/player' | ||
require './lib/game' | ||
|
||
class Multi | ||
attr_reader :players_num | ||
|
||
def initialize(player_1_name, player_2_name, players_num) | ||
@players_num = players_num | ||
@player_1_name = player_1_name | ||
@player_2_name = player_2_name | ||
end | ||
|
||
def set_players | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you include single line comments before each method. Your implementation is not trivial, and so the complexity needs to be better explained. |
||
@player_1 = Player.new(@player_1_name) | ||
if @players_num == 1 | ||
@player_2 = Player.new("Computer") | ||
else | ||
@player_2 = Player.new(@player_2_name) | ||
end | ||
end | ||
|
||
def game_creation | ||
self.set_players | ||
game = Game.new(@player_1, @player_2, players_num) | ||
return game | ||
end | ||
|
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
# lib/player.rb | ||
|
||
class Player | ||
attr_reader :name | ||
attr_accessor :choice | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great use of attr! |
||
|
||
def initialize(name) | ||
@name = name | ||
@choice = nil | ||
end | ||
|
||
def throw | ||
@choice = [:rock, :scissors, :paper].sample | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
@choice | ||
end | ||
|
||
# private | ||
|
||
# def throw | ||
# @choice = [:rock, :scissors, :paper].sample if @choice == nil | ||
# end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
# spec/features/choose_option_spec.rb | ||
|
||
feature 'Throwing page' do | ||
scenario 'Showing options' do | ||
sign_in_as_michael | ||
expect(page).to have_content 'ROCK!' | ||
expect(page).to have_content 'PAPER!' | ||
expect(page).to have_content 'SCISSORS!' | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
# spec/features/landing_page_spec.rb | ||
|
||
feature 'Landing page' do | ||
scenario 'Showing name of game' do | ||
visit('/') | ||
expect(page).to have_content 'Welcome to the Ultimate Rock Paper Scissors Game!' | ||
end | ||
|
||
scenario 'allow to choose single player or multiple player' do | ||
visit('/') | ||
expect(page).to have_content "Enter Player 1's name:" | ||
expect(page).to have_content "Enter Player 2's name:" | ||
end | ||
|
||
scenario 'Allows user to enter name' do | ||
sign_in_as_michael | ||
expect(page).to have_content 'Michael, please choose your choice.' | ||
end | ||
|
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
# spec/features/muti_player_spec.rb | ||
|
||
feature 'Two throwing page' do | ||
scenario 'Showing Player 2 name for two players (when first player choosing)' do | ||
sign_in_as_tom_and_jerry | ||
expect(page).to have_content "Tom, please choose your choice." | ||
expect(page).to have_content "Your opponent's name: Jerry" | ||
end | ||
|
||
scenario 'Showing Player 1 name for two players (when second player choosing)' do | ||
sign_in_as_tom_and_jerry | ||
choose('rock') | ||
click_button("Throw!") | ||
expect(page).to have_content "Jerry, please choose your choice." | ||
expect(page).to have_content "Your opponent's name: Tom" | ||
end | ||
|
||
scenario 'Showing options for two players one by one' do | ||
sign_in_as_tom_and_jerry | ||
expect(page).to have_content "Tom, please choose your choice." | ||
expect(page).to have_content 'ROCK!' | ||
expect(page).to have_content 'PAPER!' | ||
expect(page).to have_content 'SCISSORS!' | ||
choose('rock') | ||
click_button("Throw!") | ||
expect(page).to have_content "Jerry, please choose your choice." | ||
expect(page).to have_content 'ROCK!' | ||
expect(page).to have_content 'PAPER!' | ||
expect(page).to have_content 'SCISSORS!' | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
# spec/features/show_results_spec.rb | ||
|
||
feature 'Throwing page' do | ||
scenario 'Win' do | ||
sign_in_as_michael | ||
choose('scissors') | ||
srand(4) | ||
click_button("Throw!") | ||
expect(page).to have_content "Michael: scissors vs Computer: paper" | ||
expect(page).to have_content 'You Win!' | ||
end | ||
|
||
scenario 'Lose' do | ||
sign_in_as_michael | ||
choose('rock') | ||
srand(4) | ||
click_button("Throw!") | ||
expect(page).to have_content "Michael: rock vs Computer: paper" | ||
expect(page).to have_content 'You Lose!' | ||
end | ||
|
||
scenario 'Tie' do | ||
sign_in_as_michael | ||
choose('paper') | ||
srand(4) | ||
click_button("Throw!") | ||
expect(page).to have_content "Michael: paper vs Computer: paper" | ||
expect(page).to have_content 'It is a tie!' | ||
end | ||
|
||
scenario 'Two players - Player 1 wins' do | ||
sign_in_as_tom_and_jerry | ||
choose('rock') | ||
click_button("Throw!") | ||
choose('scissors') | ||
click_button("Throw!") | ||
expect(page).to have_content "Tom wins!" | ||
end | ||
scenario 'Two players - Player 2 wins' do | ||
sign_in_as_tom_and_jerry | ||
choose('rock') | ||
click_button("Throw!") | ||
choose('paper') | ||
click_button("Throw!") | ||
expect(page).to have_content "Jerry wins!" | ||
end | ||
scenario 'Two players - Tie' do | ||
sign_in_as_tom_and_jerry | ||
choose('rock') | ||
click_button("Throw!") | ||
choose('rock') | ||
click_button("Throw!") | ||
expect(page).to have_content "It is a tie!" | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# spec/features/web_helpers.rb | ||
|
||
def sign_in_as_michael | ||
visit('/') | ||
fill_in :player_1_name, with: "Michael" | ||
choose('1') | ||
click_button "Let's Go!" | ||
end | ||
|
||
def sign_in_as_tom_and_jerry | ||
visit('/') | ||
fill_in :player_1_name, with: "Tom" | ||
fill_in :player_2_name, with: "Jerry" | ||
choose('2') | ||
click_button "Let's Go!" | ||
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.
I love this, you've saved yourself some work here!
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.
Great idea