Cleaning up code and making better loops for soccer shoot-out game

Question

I'll start by saying I've just started with programming, and Ruby is my first language. I've made a little soccer shoot-out game. I'm having problems with the end of the code looping back to where I want it.

If there's a tie, I want it to keep the same score and players, but add another round of shooting. Should I do this by creating an array to store the points?

If there's anything else I can simplify, I'd really appreciate your input.

class Shoot
def initialize 
    shoot
end

def shoot
    @kick = 1 + rand(6)
end

def kick
    @kick
end
end

response = ''
until response == 'N'
play = "Have another shoot-out? Put Y or N."

puts "Player 1's name is:"
player_1_name = gets.chomp

puts "Player 2's name is?"
player_2_name = gets.chomp

puts "#{player_1_name} and #{player_2_name}, are you ready to play sudden death   (ENTER)?!"
gets.chomp

p1_score = 0
p2_score = 0
result_1 = 0
result_2 = 0

until result_1 >= 1 || result_2 >= 1
  puts "#{player_1_name}, please take your kick (ENTER)."
  gets.chomp!
  shot_1 = Shoot.new.kick
  result_1 = shot_1*0.2
  puts "#{player_1_name} takes the shot, and it's looking good..."
  puts "(ENTER)"
  gets.chomp

  if result_1 >= 1
      puts "...And it's a goal!!!"
      p1_score += 1
  else
      puts "...Oh! So close, but it's no good!!!"
  end

  puts "#{player_2_name}, please take your kick (ENTER)."
  gets.chomp!

  shot_2 = Shoot.new.kick
  result_2 = shot_2*0.2
      puts "#{player_2_name} takes the shot, and it's looking good..."
  puts "(ENTER)"
  gets.chomp

  if result_2 >= 1
      puts "...And it's a goal!!!"
      p2_score += 1
  else
      puts "...Oh! So close, but it's no good!!!"
  end

puts "#{player_1_name}: #{p1_score}"
puts "#{player_2_name}: #{p2_score}"

if p1_score > p2_score
    puts "#{player_1_name}, you're the winner! Congratulations!"
elsif p1_score < p2_score
    puts "#{player_2_name}, you're the winner! Congratulations!"
else
    puts "Looks like we've gotta go another round!"
    return
end
end

puts play
response = gets.chomp
end

Answer 1

Not a bad start. However here's a few things that immediately pops out when I look at the code.

1. Avoid duplication: The code for both players to kick is exactly the same. You should try to move that into a function instead of writing it out twice.
2. Make a player class: By moving all the code for a player into a class, you avoid having to keep each variable twice. Instead of shot_1/shot_2, result_1/result_2 etc, you could have player[0].shot, player[0].result etc.
3. Make a game class: While you're at it, why not also turn the game itself into a class. I think this would solve your looping problem all by itself.

Something like this:

class Game
  def initialize
    # set up for a new game
  end

  def play_round
    # play one round of the game
  end
end

Answer 2

You could also move some stuff into the Shoot class you already have,

class Shoot
  def initialize
    @kick = 1 + rand(6)
  end

  def result
    @kick * 0.2
  end

  def goal?
    result >= 1
  end
end

So then you can write

if Shoot.new.goal?
  puts "...And it's a goal!!!"
  p2_score += 1
else
  puts "...Oh! So close, but it's no good!!!"
end

The general rule here is that if you are pulling data out of an object, and then using it to do more work, it might make more sense to have that logic inside the object.

Answer 3

You asked if you should set up an array to keep track of things. I find math is sometimes much easier to keep a count of things. I do so in the code below.

I have set this up with the following (somewhat standard) folder structure:

.
|-- bin
|   `-- sudden_death
|-- lib
|   |-- player.rb
|   `-- referee.rb
`-- test
    |-- player_spec.rb
    |-- referee_spec.rb
    `-- spec_helper.rb

3 directories, 7 files

What is in bin is a Ruby file that is marked as executable, and holds this code:

#!/usr/bin/env ruby
$:.unshift 'lib/'
%w[referee player].each do |r|
  require r
end
include Referee

player_1 = Player.new 'Ford Prefect'
player_2 = Player.new 'Zaphod Beeblebrox'

puts "Are you ready to Rumble?"
until Referee::winner?(player_1, player_2)
  [player_1, player_2].each do |player|
    puts "#{player.name} kicks and... "
    rand(6) * 0.2 == 1 && player + 1
    puts "#{player.name} #{player.score == 1 ? 'scores!' : 'misses'}"
  end
end
winner = Referee::winner?(player_1, player_2).name
puts "\n\nAnd the winner is:  #{winner}!"

And a sample run:

Are you ready to Rumble?              
Zaphod Beeblebrox kicks and...·       
Zaphod Beeblebrox scores!             
Ford Prefect kicks and...·            
Ford Prefect misses                   

And the winner is:  Zaphod Beeblebrox!

This is made possible by Player and Referee

They are not both classes, as you can see in the code above.

Here is lib/player.rb

class Player
  attr_reader :score, :name
  def initialize name, handicap = 0
    @name = name
    @score = handicap
  end

  def +(other)
    @score += other
  end

  def -(other)
    @score -= other
  end
end

And here is lib/referee.rb

module Referee

  def winner?(player_1, player_2)
    case (player_1.score <=> player_2.score)
    when 0
      nil
    when 1
      player_1
    when -1
      player_2
    end
  end

end

As you move other things out of the bin/sudden_death.rb more of the game mechanics may be managed by the Referee module. There is still a lot of work to do in these files, and you will rearrange things as they make sense to you.

I spent just a few minutes on it this afternoon, but I hope it helps.