Skip to content
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

Basic RPS program - JM #257

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jordanmarkham
Copy link

Basic Rock, Paper, Scissors program - quite limited testing, but it works properly!

Copy link

@HanzAkor HanzAkor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful application. Works really well. Just add more tests to cover every possible scenario.

erb(:result)
end

def pick_winner(player, opponent)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nested if statement could be optimised better. Maybe use a hash.
Also, put this in a class file and instantiate in the app file to keep it more tidy.

scenario "When I select 'Rock', I will lose if the machine selects 'Paper'" do
allow_any_instance_of(@opponent_choice).to receive(:sample).and_return(:paper)
visit '/'
fill_in('name', with: 'Jordan')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a web_helpers page where you can create a separate method and call it here, to keep the test more tidy and avoid repeating too many lines.


<h2>You picked <%= @user_choice %>, and your opponent picked <%= @opponent_choice %></h2>

<form action="/" method="get">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redirect to the game and keep the user's name so that they don't need to keep entering it again.
Enabling sessions will help with this:
https://github.com/makersacademy/course/blob/main/apprenticeships_intro_to_the_web/walkthroughs/post_redirect_get_pattern.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants