3
\$\begingroup\$

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"
\$\endgroup\$
  • \$\begingroup\$ Ruby is the worst language for this problem. \$\endgroup\$ – Nakilon Aug 29 '13 at 9:38
  • \$\begingroup\$ @Nakilon Wow, that's a doozy. Please explain. \$\endgroup\$ – Mark Thomas Aug 29 '13 at 10:05
  • \$\begingroup\$ @MarkThomas, chosing the language for fast processing binary arrays? You really think it needs explanation? \$\endgroup\$ – Nakilon Aug 29 '13 at 12:14
  • \$\begingroup\$ @Nakilon I think you're making assumptions about the problem. You may be correct, but on the other hand, I've seen implementations in Perl, PHP, JS (none of which are considered particularly speedy languages) as well as Ruby which were perfectly fit for their purpose. Maybe it was just a learning exercise. \$\endgroup\$ – Mark Thomas Aug 29 '13 at 19:33
  • \$\begingroup\$ James, if @David has answered your question to your satisfaction, you should mark his answer as 'accepted'. \$\endgroup\$ – Cary Swoveland Sep 29 '13 at 7:41
3
\$\begingroup\$
  • 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.

\$\endgroup\$
  • \$\begingroup\$ Awesome, thank you very much. This alone helped me organize much more effectively. \$\endgroup\$ – James Carny Sep 4 '13 at 15:10
  • \$\begingroup\$ "Ruby doesn't have tail recursion, so you shouldn't do recursive calls without end conditions" - Doesn't a recursive method need an end condition whether or not the language has tail call optimization? \$\endgroup\$ – Wayne Conrad Oct 2 '13 at 17:41
  • \$\begingroup\$ @WayneConrad: Normally yes, hence my question "Iterate till ^C?". The end condition here is implicitly given by the signal handling of the process. But even then we don't know the limit of the number of iterations/recursions which might occur, so the loop should be implemented in a way which doesn't leak memory or fills the stack. \$\endgroup\$ – David Ongaro Oct 22 '13 at 1:31

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.