Cleaning up code for Conway's Game of Life

Question

I've implemented a Ruby version of Conway's Game of Life that functions correctly and passes all my tests. I was hoping to get some advice on how I could clean up the code for readability and efficiency.

For anyone who doesn't know the rule to the Game of life, they are as follows:

• Any live cell with fewer than two live neighbours dies, as if by needs caused by underpopulation.
• Any live cell with more than three live neighbours dies, as if by overcrowding.
• Any live cell with two or three live neighbours lives, unchanged, to the next generation.
• Any dead cell with exactly three live neighbours becomes a live cell.

(In my implementation, 1 is an alive cell and 0 is a dead cell).

class Game_of_life
  def initialize(hash)
    @test = nil
    str = hash[:string]
    size = hash[:size]
    @length = nil
    @board = []
    if str != nil
      str.each_char {|c| @board << c.to_i}
      @test = true
      @length = Math.sqrt(str.length)
    else
      (size*size).times do
        @board << [0,1].sample
      end
      @length = size
    end
  end

  def check_board

    @board.each_with_index do |organism, index|
      neighbours = {alive: 0, dead: 0}
      neighbour_positions = choose_neighbour_set(index)
      neighbour_positions.each do |position|
        neighbours[:alive] += 1 if @board[index+position] == 1
        neighbours[:dead] += 1 if @board[index+position] == 0
      end
      @board[index] = 0 if organism == 1 && (neighbours[:alive] < 2 || neighbours[:alive] > 3)
      @board[index] = 1 if organism == 0 && neighbours[:alive] == 3
    end
    if @test
      return @board.join("")
    else
      sleep 0.5
      puts "\e[H\e[2J"
      @board.each_slice(@length) {|row| p row }
      check_board
    end
  end

  def choose_neighbour_set(index)
    neighbour_set = [-(@length), -(@length - 1), 1, (@length+1), @length, (@length-1), -1, -(@length + 1)]
    neighbour_set.delete_if{|position| [-(@length + 1), -(@length),-(@length - 1)].include?(position)} if index < @length
    neighbour_set.delete_if{|position| [(@length-1),@length,(@length+1)].include?(position)} if index > ((@length*(@length-1)) -1)
    neighbour_set.delete_if{|position| [-(@length + 1),-1,(@length-1)].include?(position)} if index % @length == 0 || index == 0
    neighbour_set.delete_if{|position| [(@length+1),1,-(@length - 1)].include?(position)} if (index + 1) % @length == 0 && index != 0
    return neighbour_set
  end
end


# Uncomment to watch the game of life unfold!
# Game_of_life.new({size: 25}).check_board


# >>>>>>>>>>>>>>>>>>>>> Driver code

# Test string one
arr = "100100111110100011110110001100110010000010111101100010010111110110111100110000011"
game = Game_of_life.new({string: arr}) 
p game.check_board == "110000101000111101110000101111110001100011101101000000000000110111111001101011111"

# test string two
arr2 = "110111000101110010011000100110100100000101110111001110111011011101110010010110110"
game2 = Game_of_life.new({string: arr2})
p game2.check_board == "110011000100110110001010100001010010000100011101001000101001011100010001000111110"

# test string three
arr3 ='100110010'
game3 = Game_of_life.new({string: arr3})
p game3.check_board == "110011011"

# test string four

arr4 = "0010010111001001"
game4 = Game_of_life.new({string: arr4})
p game4.check_board == "0010101110100000"

Answer 1

• Ruby convention is to use CamelCase for Classnames (GameOfLife)
• Since "size" and "str" are mutably exclusive parameters, using a hash parameter here makes the API confusing
• Using flags like "@test" to define object behavior is bad style (antipattern)
• Ruby doesn't have tail recursion, so you shouldn't do recursive calls without end conditions

To make this more OOP like and self documenting you can define a base class for the basic initialization:

class GameOfLife
  def initialize(length)
    @length = length # Why not call this "size"?
    @board = []
  end

  def check_board
    # Implemented as above without the "if" statement
  end

  def choose_neighbour_set(index)
    # As above
  end
end

Then you can use subclasses for specific implementations

class RandomGameOfLife < GameOfLife
  def initialize(size)
    super(size)
    (size*size).times do
      @board << [0,1].sample
    end
  end

  def iterate_and_output_board
    while true # Iterate till ^C?
      check_board
      sleep 0.5
      puts "\e[H\e[2J"
      @board.each_slice(@length) {|row| p row }
    end
  end
end

class PredefinedGameOfLife < GameOfLife # Or use a better name here
  def initialize(str)
    super(Math.sqrt(str.length))
    str.each_char {|c| @board << c.to_i}
  end

  def check_board
    # This is just an example how to overwrite method behavior
    # Defining another method using "check_board" might be better here
    super
    @board.join("")
  end
end

Somebody else might cover the efficiency aspect. I don't want to intermix the aspects here and its better to focus on readability first.