Ruby implementation of Conway's Game of Life

Question

I'm just starting with Ruby and decided to do two different implementations of Conway's Game of Life. In this first one I'm not using cells as a separate class and instead just track everything as a location on the board, using a state pattern I guess.

Anyway, I want this code to display good Object-Oriented programming basics as well as construction via test-driven development, using rspec.

Please critique away and tell me if it's under/over engineered for the problem, not extensible enough or too extended, as well as if my TDD progression was worthwhile, did I do enough or too few tests, or maybe not the right tests.

board.rb:

class Board
attr_accessor :width,:height

  def initialize(cells=[], width = 3, height = 3)
    @cells = cells
    @height = height
    @width = width
  end

  def live_cells
    @cells
  end

  def is_alive?(x,y)
    @cells.include?([x,y])
  end

  def neighbors_to(x,y)
    @cells.select do |cell_x, cell_y|
    dx = (x-cell_x).abs
    dy = (y-cell_y).abs
    #If the tuple x,y is a neighbor of that cell then the following is true
    dx <= 1 and dy <= 1 and not (dx == 0 and dy == 0)
    end.length
    #We only care how many there are and do not care which cells in particular
  end

  def evolve
    survivors = []
    #Scan the entire board (may be able to be more clever here but you need to check each dead cell for ressurection)
    @width.times do |x|
      @height.times do |y|
        survivors << [x,y] if should_survive?(x,y) or should_ressurect?(x,y)
      end
    end
    #Populating the new board from survivors allows reference to the current generation while populating the next generation
    Board.new(survivors, self.width,self.height)
  end

  def should_survive?(x,y)
    (neighbors_to(x,y) == 2 or neighbors_to(x,y) == 3) and is_alive?(x,y)
  end

  def should_ressurect?(x,y)
    neighbors_to(x,y) == 3 and not is_alive?(x,y)
  end

  def print_board
    puts "Current Generation"
    puts
    @width.times do |x|
      @height.times do |y|
        print "#{is_alive?(x,y) ? '#' : '-'}"
      end
      puts
    end
    puts
  end

end

board_spec.rb:

require 'spec_helper'

describe Board do

  context "#new" do
    it "should be created empty if passed empty array" do
      Board.new([]).live_cells.should be_empty
    end

    it "should be created empty if passed nothing at all" do
      Board.new
    end

    it "should contain the cells created with it" do
      Board.new([[1,1],[2,2]]).live_cells.should be ==[[1,1],[2,2]]
    end

  end

  context "#evolve" do

    it "should evolve an empty board if empty" do
      next_gen = next_generation_for([])
      next_gen.should be_empty
    end

    it "should kill a lonely cell (< 2 neighbors)" do
      next_gen = next_generation_for([[1,1]])
      next_gen.should be_empty
    end

    it "should keep a cell with 2 neighbors alive" do
      #pending("We need to count neighbors")
      next_gen = next_generation_for([[1,1],[1,2],[1,0]])
      next_gen.should include([1,1])
    end

    it "should keep a cell with 3 neighbors alive" do
      #pending("We need to count neighbors")
      next_gen = next_generation_for([[1,1],[1,2],[1,0],[0,0]])
      next_gen.should include([1,1])
    end

    it "should revive a dead cell with 3 neighbors (reproduction)" do
      #pending("Dimensions of the board to find dead cells")
      next_gen = next_generation_for([[1,0],[0,1],[2,1]])
      next_gen.should include([1,1])
    end

    it "should kill any cell with more than 3 neighbors" do
      next_gen = next_generation_for([[0,1],[2,1],[1,2],[1,0],[1,1]])
      next_gen.should_not include([1,1])
    end
  end

  def next_generation_for(seed,width = 3, height = 3)
    board = Board.new(seed, width, height)
    board.evolve.live_cells
  end


  it "should be able to handle the blinker formation 10 times" do
    seed = [[1,2],[1,1],[1,0]]
    n = 0
    while n<=10
      next_gen = next_generation_for(seed)
      next_gen.should satisfy{|cells| cells =~[[0,1],[1,1],[2,1]] or [[1,2],[1,1],[1,0]]}
      seed = next_gen
      n+=1
    end
  end

  it "should be able to handle the block formation 10 times" do
    seed = [[1,1],[2,1],[2,2],[1,2]]
    n = 0
    while n <= 10
      next_gen = next_generation_for(seed)
      next_gen.should =~ [[1,1],[2,1],[2,2],[1,2]]
      seed = next_gen
      n+=1
    end
  end

  it "should be able to handle the beehive formation 10 times" do
    seed = [[1,0],[2,0],[3,1],[2,2],[1,2],[0,1]]
    n = 0
    while n <= 10
      next_gen = next_generation_for(seed,4,4)
      next_gen.should =~ [[1,0],[2,0],[3,1],[2,2],[1,2],[0,1]]
      seed = next_gen
      n+=1
    end
  end

  it "should be able to print the current board" do
    board = Board.new([[1,1],[0,1],[0,2]],5,5)
    board.print_board
  end
end

describe Board, "counting neighbors" do

  it "should find zero neighbors to a solitary cell" do
    board = Board.new([[1,1]])
    board.neighbors_to(1,1).should == 0
  end

  it "should find one neighbor to a single cell" do
    board = Board.new([[1,1]])
    board.neighbors_to(0,0).should == 1
    board.neighbors_to(0,1).should == 1
    board.neighbors_to(0,2).should == 1
    board.neighbors_to(1,2).should == 1
    board.neighbors_to(2,2).should == 1
    board.neighbors_to(2,1).should == 1
    board.neighbors_to(2,0).should == 1
    board.neighbors_to(1,0).should == 1
  end

  it "should find two neighbors to cells if needed" do
    board = Board.new([[0,1],[2,2]])
    board.neighbors_to(1,1).should == 2
    board.neighbors_to(1,2).should == 2
  end

  it "should find three neighbors to cells if needed" do
    board = Board.new([[0,1],[2,2],[1,2]])
    board.neighbors_to(1,1).should == 3
    board.neighbors_to(2,2).should == 1
  end

end

spec_helper just has a pointer to require board.rb. I'm trying to get into this practice for larger programs so adding new classes won't require me to edit individual spec files. I also have a file, run_game.rb, that simply allows me to choose different patterns and iterates the game.

Some things that stand out to me:

• There is very little user fault tolerance, i.e. I can give it cells on a board that's too small and it runs just fine, it just never displays cells off the board. Is it worth the extra complication to solve such problems, i.e. make it more durable to a user or is this structure enough to communicate a foundation understanding of OO programming?
• Is there repetition in the live_cells method? I could just use attr_accessor :cells but that's far less readable.
• Would it be a "better" implementation from an OO programming standpoint to have cells as a class as well?

Answer 1

Your code looks pretty good to me. I specially like how you don't update inplace a Board but create new ones on each iteration. And the OOP aspects of it seem also ok, I wouldn't write a Cell class when a pair [x, y] does just fine and there are not many methods you can encapsulate within (other than dead/alive, which isn't helpful as your existing cells are those alive). Some notes on details:

• def initialize(cells=[], width = 3, height = 3). I wouldn't recommend positional optional arguments, it gets messy pretty quickly. Maybe def initialize(options = {}) with key arguments? Note that Ruby 2.0 has keyword arguments: def initialize(cells: [], width: 3, height: 3)

• def is_alive?(x,y) Add some spaces to let it breathe, it enhances readability: def is_alive?(x, y)

• dx <= 1 and dy <= 1 and not (dx == 0 and dy == 0). and/or are used for control flow, &&/|| for boolean expressions. And ! is more idiomatic than not.

• @cells.select { ... }.length note that this may be written @cells.count { ... }.

• survivors = []. Your code is almost functional, don't give up:

  survivors = (0...@width).to_a.product(0...@height).select do |x, y|
    should_survive?(x, y) || should_ressurect?(x, y)
  end
  

• print_board. I'd write it differently (also in functional style):

  def print_board
    matrix = 0.upto(@width - 1).map do |x|
      0.upto(@height - 1).map do |y|
        is_alive?(x, y) ? '#' : '-'
      end.join
    end.join("\n")
    puts "Current Generation\n\n#{matrix}\n"
  end
  

Answer 2

I won't talk about style, because @tokland's comments are spot-on.

However, some quick notes about efficiency. For large board sizes, this implementation is inefficient.

For example, neighbors_to scans every cell on the board, and it is called multiple times. It is called twice from the should_survive? method:

def should_survive?(x,y)
  (neighbors_to(x,y) == 2 or neighbors_to(x,y) == 3) and is_alive?(x,y)
end

You can reduce the call to one, and swap the order so you don't have to call it at all if the cell is alive:

def should_survive?(x,y)
  is_alive?(x,y) and [2,3].include? neighbors_to(x,y)
end

The implementation of neighbors_to should simply collect each of the surrounding eight (maximum) spaces rather than scan the entire set of live cells.

This also means you need to be able to query a cell directly, rather than scan all live cells as your is_alive? method currently does. I would recommend representing a board using a 2D array of spaces. I'd probably rename @cells to @live_cells to avoid confusion.