# Rock, Paper, Scissors game

I am very new to Ruby and building my first game. To be honest I did it this way because I was lazy to type so much. My friend told me that the correct way was to do this with case statement i.e. for scissor we have three cases, for rock we have three cases and similarly for paper.

puts "Rock, paper or scissor"
w = 0 and l =0 and t =0 and j=0
loop{
choice = ["paper" , "rock" , "scissor" ]
i=0

if j ==10
puts "wins : #{w}\nlosses : #{l}\ndraw:    #{t}"
if w > l
puts "You won the game"
elsif w<l
puts "You lost!"
else
puts "The game is a draw"
end

gets
break
end
while i == 0
human = gets.chomp.downcase
choice.each {
|x|
if human == x
i+=1 and j+=1
break
end
}
puts "-----------------please enter rock paper or scissor-----------------" if i == 0
end
computer = rand(3)
com = choice[computer]
if (com == "scissor" and human != "rock") or (human == "scissor" and computer !=1)
v = human <=> com
else
v = com <=> human
end

puts "You chose #{human}, computer choose #{com}"
case v

when 1
puts "---->You win"
w +=1
when -1
puts "---->Computer wins"
l+=1
else
puts "---->draw"
t+=1
end
}

• can you fix the indenting? – tokland Apr 29 '14 at 15:39

Indentation
Be sure to indent your code properly. Indentation helps the reader understand where he is in the code (how deep). Some languages actually depend on correct indentation.

Meaningful names
You should give your variable names which will have meaning, and will help the reader of your code understand what each one is responsible for. Names like w, t, j should not be used for any variable other than immediate throw-away variables like if for(i = 1;....

A variable name l should never ever be used, since it is too easily confused with the digit 1, so in some fonts you will never know the difference between l+=1 and l+=l...

Your code should tell a story
Try to design your code in a way that a reader will be able to follow your logic. In your code, there is an endless loop, and in the first if you check whether a mysterious j is 10, and if it is you end the game... The story starts with the ending!

A more pleasing option might be:

while number_of_turns_played < 10 do

# play a turn...
number_of_turns_played += 1

end

if number_of_wins > number_of_losses
puts "You won the game"
elsif number_of_wins < number_of_losses
puts "You lost the game"
else
puts "The game is a draw"
end


This way the story has a beginning, and an end, and it easier to follow.

What is i used for? As far as I can see it can have only two values - 0 and 1, and it is used to flag the get input loop that a valid input has been received. The name i is cryptic enough - why not at least make it a boolean?

Block format conventions
It is idiomatic in ruby to use {} syntax for one liner blocks, and do end syntax for multiline blocks.

choice.each {
|x|
# do something
}


You should write:

choice.each do |x|
# do something
end


Too sophisticated for your own good
Let's look at the condition you worked so hard on for being lazy:

if (com == "scissor" and human != "rock") or (human == "scissor" and computer !=1)
v = human <=> com
else
v = com <=> human
end

1. First - be consistent - on one side check com and on the other you check computer - choose!
2. Using esoteric operators - the "spaceship" operator (a<=>b) is used when sorting arrays, and returns -1 if a is smaller than b, 1 if a is larger than b, and 0 if they are equal. It is very rarely used, and your usage, while cute, might be very unclear for most (I know I had to double check what it returns). Is it really needed?
3. Assumptions - you rely on the fact that "paper" is smaller than "rock", which is smaller than "scissor", and you write your algorithm around that assumption. You are relying on an incidental fact, which might change when, say, you translate it to another language! Don't make incidental assumptions, use your code to define your rules.
4. When there are too many exceptions to the rule - so you found a cool rule, which is good for [almost] all your cases. That is all but two, out of six... which is four... which is just about half the cases... Which you will need to explain to anybody who reads the code, since it is not apparent from how the code is written... Maybe it is not that good a rule?

So, what would I suggest?

Say I set the choices to %w(rock paper scissors), where each choice beats the one prior to it, now I can ask:

computer_choice_idx = rand(3)
if human_choice == choices[computer_choice_idx]
# draw!
elsif human_choice == choices[computer_choice_idx - 1]
# computer beats human!
else
# human beats computer!
end


Since in ruby choice[-1] returns the last element in the array ("scissors"), there is no exception! If the computer chose "rock"(id 0) and the human chose "scissors" - computer will beat human, since "scissors" == choices[-1].

In short:

Choices = %w(rock paper scissors)

puts 'Rock, paper or scissors'
draws = wins = losses = 0
10.times do
begin
puts "-----------------please enter rock paper or scissors-----------------"
human_choice = gets.chomp.downcase
end until Choices.include?(human_choice)

computer_choice_idx = rand(3)

puts "You chose #{human_choice}, computer choose #{Choices[computer_choice_idx]}"

if human_choice == Choices[computer_choice_idx]
puts "---->draw"
draws += 1
elsif human_choice == Choices[computer_choice_idx - 1]
puts "---->Computer wins"
losses += 1
else
puts "---->You win"
wins += 1
end
end
puts "wins : #{wins}\nlosses : #{losses}\ndraw:    #{draws}"
if wins > losses
puts "You won the game"
elsif wins < losses
puts "You lost!"
else
puts "The game is a draw"
end


Enjoy!

You can initialize the score's var this way : w = l = t = j = 0.

Choices could be initilize with the literal string array : choice = %w( rock paper scissor ).

Don't use the and reserved key word to inline code, you can use the ; but multilines is better. Same for boolean and and or, use && and ||.

When you wrote a multilines block, use the do ... end syntax :

choice.each {
|x|
if human == x
i+=1 and j+=1
break
end
}


Becomes :

choice.each do |x|
if human == x
i+=1
j+=1
break
end
end


And you can use the Array#include? method to check the player's input :

if choice.include?(human)
i += 1
j += 1
end


Instead of doing this :

computer = rand(3)
com = choice[computer]


You can use the Array#sample method like that : com = choice.sample

You should use Symbol instead of String for choices : "scissor" become :scissor.

Here's my version :

puts "Rock, paper or scissor"
w = l = t = j = 0 # inline instanciation
# instanciate choice outside the loop otherwise it is recreate each turn
choice = %i(paper rock scissor) # use a literal symbols array to define choices

loop do # use do...end instead of {} for multilines
if j == 10
puts "wins : #{w}\nlosses : #{l}\ndraw:    #{t}"
if w > l
puts "You won the game"
elsif w < l
puts "You lost!"
else
puts "The game is a draw"
end

gets
break
end

human = nil
until choice.include?(human) # use of until and choice.include? to loop since human make a valid choice
puts "Do your choice (paper, rock, scissor) :"
human = gets.chomp.downcase.to_sym # String#to_sym convert string in symbol
end

com = choice.sample # use of Array#sample to make a random choice between all
puts "You chose #{human}, computer choose #{com}"
# use a simple if..else to check result which is more readable
if (com == human)
t += 1
puts "---->draw"
elsif (com == :scissor && human == :paper) || # details all com's wins is more expressif
(com == :paper && human == :rock)    ||
(com == :rock && human == :scissor)
l += 1
puts "---->Computer wins"
else
w += 1
puts "---->You win"
end

j+=1 # the end of turn is here, so increment here
end