2
\$\begingroup\$

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
}
\$\endgroup\$
  • \$\begingroup\$ can you fix the indenting? \$\endgroup\$ – tokland Apr 29 '14 at 15:39
3
\$\begingroup\$

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.

Choose your types
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.
Instead of writing:

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!

\$\endgroup\$
2
\$\begingroup\$

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
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.