Please Review My First Ruby Program

[Simeon Willbanks]

I've written a simple program to simulate a Soccer game. Its the first
full program I've written in Ruby, so please forgive any obvious
mistakes. In advance, thanks for your comments. It requires Ruby
1.9. Please run the unit tests to see the program in action.

Simeon

$ ruby TestSoccerGame.rb

---

### TestSoccerGame.rb
require "SoccerGame"
require "test/unit"

class TestSoccerGame < Test::Unit::TestCase

# Més que un club
FC_BARCELONA = {
'club' => 'FC Barcelona',
'players' => [
[1, 'Victor Valdes Arribas', :keeper],
[20, 'Daniel Alves da Silva', :defender],
[3, 'Gerard Pique Bernabeu', :defender],
[4, 'Rafael Marquez Alvarez', :defender],
[5, 'Carles Puyol Saforcada', :defender],
[6, 'Xavier Hernandez Creus', :midfielder],
[8, 'Andres Iniesta Lujan', :midfielder],
[15, 'Seydou Keita', :midfielder],
[9, 'Zlatan Ibrahimovic', :striker],
[10, 'Lionel Andres Messi', :striker],
[14, 'Thierry Henry', :striker]
]
}

# You'll Never Walk Alone
LIVERPOOL_FC = {
'club' => 'Liverpool FC',
'players' => [
[25, 'Pepe Reina', :keeper],
[2, 'Glen Johnson', :defender],
[23, 'Jamie Carragher', :defender],
[37, 'Martin Skrtel', :defender],
[22, 'Emiliano Insua', :defender],
[20, 'Javier Mascherano',:midfielder],
[8, 'Steven Gerrard', :midfielder],
[11, 'Albert Riera', :midfielder],
[15, 'Yossi Benayoun', :midfielder],
[9, 'Fernando Torres', :striker],
[18, 'Dirk Kuyt', :striker]
]
}

def setup
if RUBY_VERSION.to_f < 1.9
flunk("SoccerGame requires Ruby 1.9 or greater")
end

# @g used for all tests except full game test
@g = SoccerGame.new FC_BARCELONA, LIVERPOOL_FC
end

# Full game test
def test_champions_league_final
@f = SoccerGame.new FC_BARCELONA, LIVERPOOL_FC
@f.title = "UEFA Champions League Final"

# Starting lineups
assert @f.home
assert @f.away
assert_equal 11, @f.home.players.length
assert_equal 11, @f.away.players.length

# Kickoff
assert @f.eventsfirst_half_kickoff)
assert @f.first_half
assert_equal false, @f.second_half

# Show Alves yellow
assert_block "Dani Alves" do
@f.yellow_cardhome, 20)
end
assert_equal 1, @f.yellow_cards[:home].length

# El Nino sneaks one in
@f.goalaway, 9)
assert_equal 1, @f.score[:away]
assert_equal 1, @f.goals[:away][9]

# Halftime
assert @f.eventshalf_time)
assert_equal false, @f.first_half
assert_equal false, @f.second_half

# Start second half
assert @f.eventssecond_half_kickoff)
assert @f.second_half
assert_equal false, @f.first_half

# Show Skrtel red
away_team_length = @f.away.players.length
assert_block "Skrtel wasn't sent to the showers" do
@f.red_cardaway, 37)
end
assert_equal away_team_length-1, @f.away.players.length
assert_equal 1, @f.red_cards[:away].length

# Xavi ties the match
@f.goalhome, 6)
assert_equal 1, @f.score[:home]
assert_equal 1, @f.goals[:home][6]

# Messi seals the victory
@f.goalhome, 10)
assert_equal 2, @f.score[:home]
assert_equal 1, @f.goals[:home][10]

# Fulltime
assert @f.eventsfinal)
assert_equal false, @f.second_half
assert_equal false, @f.first_half

# Champions League Final stats
puts @f.stats
end

# A home and away team are required
def test_home_away
assert @g.home
assert @g.away
end

# Both home and away teams must have 11 players
def test_starting_lineups
assert_equal 11, @g.home.players.length
assert_equal 11, @g.away.players.length
end

# Positions normally have at least a certian number players
def test_players_per_position
assert_equal 1, @g.home.keepers.length
assert_operator 3, :<=, @g.home.defenders.length
assert_operator 3, :<=, @g.home.midfielders.length
assert_operator 1, :<=, @g.home.strikers.length
assert_equal 1, @g.away.keepers.length
assert_operator 3, :<=, @g.away.defenders.length
assert_operator 3, :<=, @g.away.midfielders.length
assert_operator 1, :<=, @g.away.strikers.length
end

def test_first_half_kickoff
assert @g.eventsfirst_half_kickoff)
assert @g.first_half
assert_equal false, @g.second_half
end

def test_half_time
assert @g.eventshalf_time)
assert_equal false, @g.first_half
assert_equal false, @g.second_half
end

def test_second_half_kickoff
assert @g.eventssecond_half_kickoff)
assert @g.second_half
assert_equal false, @g.first_half
end

def test_final_whistle
assert @g.eventsfinal)
assert_equal false, @g.second_half
assert_equal false, @g.first_half
end

def test_cards
# Show Pique yellow
assert_block "Pique isn't cautioned" do
@g.yellow_cardhome, 3)
end
assert_equal 1, @g.yellow_cards[:home].length

# Show Skrtel red
away_team_length = @g.away.players.length
assert_block "Skrtel wasn't sent to the showers" do
@g.red_cardaway, 37)
end
assert_equal away_team_length-1, @g.away.players.length
assert_equal 1, @g.away.showers.length
assert_equal 1, @g.red_cards[:away].length

# Show Pique second yellow then red
home_team_length = @g.home.players.length
assert_block "Pique wasn't sent off" do
@g.yellow_cardhome, 3)
end
assert_equal home_team_length-1, @g.home.players.length
assert_equal 1, @g.red_cards[:home].length
end

def test_scoring_a_goals
# Messi opens the scoring
@g.goalhome, 10)
assert_equal 1, @g.score[:home]
assert_equal 1, @g.goals[:home][10]

# El Nino sneaks one in
@g.goalaway, 9)
assert_equal 1, @g.score[:away]
assert_equal 1, @g.goals[:away][9]

# Ibra is so smooth
@g.goalhome, 9)
assert_equal 2, @g.score[:home]
assert_equal 1, @g.goals[:home][9]

# Messi seals the victory
@g.goalhome, 10)
assert_equal 3, @g.score[:home]
assert_equal 2, @g.goals[:home][10]
end

end

### SoccerGame.rb
require "SoccerTeam"
require "SoccerStats"

class SoccerGame

attr_accessor :title

attr_reader :home, :away,
:yellow_cards, :red_cards,
:clock, :score, :goals,
:first_half, :second_half

def initialize(home, away)
@home = SoccerTeam.new(home['club'],home['players'])
@away = SoccerTeam.new(away['club'],away['players'])
@yellow_cards = {:home => [], :away => []}
@red_cards = {:home => [], :away => []}
@score = {:home => 0, :away => 0}
@goals = {:home => {}, :away => {}}
@first_half = false
@second_half = false
end

def stats
s = SoccerStats.new
stats = "\n"+s.title(self.title)
stats << s.teams(@home.club, @away.club)
stats << s.final_score(@score[:home], @score[:away])
stats << "\n"+s.cardsyellow, @yellow_cards[:away], @away)
stats << s.cardsyellow, @yellow_cards[:home], @home)
stats << s.cardsred, @red_cards[:away], @away)
stats << s.cardsred, @red_cards[:home], @home)
stats << s.scorers(@goals, @home, @away)
stats
end

def events(whistle)
case whistle
when :first_half_kickoff then @first_half = true
when :half_time then @first_half = false
when :second_half_kickoff then @second_half = true
when :final then @second_half = false
end
@clock = Time.now.to_i
end

def count_players_on_pitch
msg = "team doesn't have the right number of players on the pitch"
if @home.players.length != 11
raise "Home" + msg
elsif @away.players.length != 11
raise "Away" + msg
end
end

def yellow_card(team, player)
# Second yellow card so, show a red
if @yellow_cards[team].index(player)
@yellow_cards[team].push player
red_card(team, player)
else
@yellow_cards[team].push player
end
end

def red_card(team, player)
@red_cards[team].push player
if team == :home
# Send player to the showers
@home.showers[player] = @home.players[player]
@home.players.delete(player)
else
@away.showers[player] = @away.players[player]
@away.players.delete(player)
end
end

def goal(team, player)
@score[team] += 1
if @goals[team][player]
@goals[team][player] += 1
else
@goals[team][player] = 1
end
end

end

### SoccerTeam.rb
class SoccerTeam

attr_accessor :showers

attr_reader :club, layers,
:keepers, :defenders, :midfielders, :strikers

def initialize(club, players)
@club = club
@players = {}
@keepers = {}
@defenders = {}
@midfielders = {}
@strikers = {}
@showers = {}
players.each do |number, name, position|
@players[number] = name
# Push player into position
case position
when :keeper then @keepers[number] = name
when :defender then @defenders[number] = name
when :midfielder then @midfielders[number] = name
when :striker then @strikers[number] = name
end
end
end

end

### SoccerStats.rb
class SoccerStats

def title(name)
self.own_line(name)
end

def teams(home,away)
self.own_line(away+" visiting "+home)
end

def final_score(home,away)
self.own_line(away.to_s+" / "+home.to_s)
end

def cards(type, cards, team)
str = ""
if !cards.empty?
if (type == :yellow)
str << "Yellow"
# Player hasn't been deleted from the team
# He was only shown a yellow card
player_hash = team.players
else
str << "Red"
# Player was deleted from the team
# He was shown a red card
player_hash = team.showers
end
str << " Cards:\n"

cards.each do |number|
str << number.to_s+"/"+player_hash[number]+"/"+team.club+"\n"
end

str = self.own_line(str)
end
str
end

def scorers(goals, home, away)
str = ""
if !goals.empty?
# Loop through each teams' goals, and print player number, name
and goals scored
{:home=>home,:away=>away}.each do |key, team|
if goals.has_key?(key)
goals[key].each do |number, goals|
str << number.to_s+"/"+team.players[number]+"/"+team.club+"
scores "+goals.to_s+" goal"
if goals > 1
str << "s"
end
str << "\n"
end
end
end
str = self.own_line("Goals:")+self.own_line(str)
end
str
end

def own_line(str)
str+"\n"
end
end

 

[David A. Black]

Hi Simeon --

I've written a simple program to simulate a Soccer game. Its the first
full program I've written in Ruby, so please forgive any obvious
mistakes. In advance, thanks for your comments. It requires Ruby
1.9. Please run the unit tests to see the program in action.

Your code looks very nice and overall very idiomatic. I've got a few
things to suggest. They're not necessarily things you urgently need to
do, but I think they'll interest you as things to think about for this
or future programs.

### SoccerGame.rb
require "SoccerTeam"
require "SoccerStats"

class SoccerGame

attr_accessor :title

attr_reader :home, :away,
:yellow_cards, :red_cards,
:clock, :score, :goals,
:first_half, :second_half

def initialize(home, away)
@home = SoccerTeam.new(home['club'],home['players'])
@away = SoccerTeam.new(away['club'],away['players'])
@yellow_cards = {:home => [], :away => []}
@red_cards = {:home => [], :away => []}
@score = {:home => 0, :away => 0}
@goals = {:home => {}, :away => {}}

If you change that to:

@goals = { :home => Hash.new(0), :away => Hash.new(0) }

then later, you can do:

def goal(team, player)
@score[team] += 1
@goals[team][player] += 1
end

without having to test for the presence of the player key.

This method:

def yellow_card(team, player)
# Second yellow card so, show a red
if @yellow_cards[team].index(player)
@yellow_cards[team].push player
red_card(team, player)
else
@yellow_cards[team].push player
end
end

could be:

def yellow_card(team, player)
if @yellow_cards[team].include?(player)
red_card(team, player)
end
@yellow_cards[team].push player
end

which represents the logic a little more cleanly, I think.

This one:

def red_card(team, player)
@red_cards[team].push player
if team == :home
# Send player to the showers
@home.showers[player] = @home.players[player]
@home.players.delete(player)
else
@away.showers[player] = @away.players[player]
@away.players.delete(player)
end
end

could conceivably be this:

def red_card(team, player)
@red_cards[team].push player
send(team).send_to_showers(player)
end

However, you have to write a send_to_showers method in the SoccerTeam
class:

def send_to_showers(player)
showers[player] = players.delete(player)
end

That's actually good anyway, because it puts the business logic of the
team in the Team class. (Note that Hash#delete returns the deleted
value, so you can assign it directly to the other hash in one
operation.)

As for the "send(team)" bit: what I'm doing is taking advantage of the
fact that you have "home" and "away" reader attributes, and your team
variable is (we hope) either :home or :away. So I'm sending that
symbol to myself... and that will return either my home team or my
away team. That way, you don't have to inline essentially the same
code twice.

The other thing I played around with was creating a SoccerPlayer
class, and adding SoccerPlayer objects to the players hash in the team
object. Ultimately I think that's a good idea, so that players can be
traded, queried for career stats outside of one particular team, etc.

Finally, I'd recommend splitting out the constants, so that you have:

module Soccer
class Team

and so on. It's a little more idiomatic, and makes the namespacing
clear.


David

--
The Ruby training with D. Black, G. Brown, J.McAnally
Compleat Jan 22-23, 2010, Tampa, FL
Rubyist http://www.thecompleatrubyist.com

David A. Black/Ruby Power and Light, LLC (http://www.rubypal.com)

 

[Simeon Willbanks]

David,

I appreciate the feedback. I'll work on incorporating your suggestions,
and I'll repost the results.

Thanks,
Simeon