12
\$\begingroup\$

This is a simple Game of Life implementation, with one neat gimmick: You can specify the rules that the simulator uses to decide whether cells should live or die. Congrats, God!

I'd especially like tips on:

  • Idiomaticity: As always, I abuse language features, and I'd like help doing that.
  • Efficiency: After a certain point, it takes a while to print out the next generation -- whether this is from figuring it out or printing it, I'm not sure. Any tips to speed up both phases are appreciated.
  • Edge cases: I tested a few common patterns (spaceships, bombs, etc.) in various rules, but I don't know enough about cellular automata to make sure it works in every scenario. I can't imagine why it wouldn't, though.
  • Prettiness: The output is kinda clunky. I'm open to improvement in the format of that as well.

game_of_life.rb

class Hash
  def boundaries
    return [[0, 0], [0, 0]] if self.empty?
    x_bounds = self.keys.inject([self.keys[0][0]] * 2) { |(min, max), (x, _)| [min, x, max].minmax }
    y_bounds = self.keys.inject([self.keys[0][1]] * 2) { |(min, max), (_, y)| [min, y, max].minmax }
    [[x_bounds[0], y_bounds[0]], [x_bounds[1], y_bounds[1]]]
  end

  def hmap(&block)
    Hash[map { |k, v| block.call(k, v) }]
  end
end

class Array
  def neighbors
    range = (-1..1).to_a
    range.product(range)
        .reject { |item| item == [0,0] }
        .map do |loc|
      loc.zip(self).map { |arr| arr.inject(:+) }
    end
  end

  def count
    self.each_with_object(Hash.new 0) do |item, count|
      count[item] += 1
    end
  end
end

def iterate(board, rules)
  board if board.empty?
  board.keys.map(&:neighbors).flatten(1).count
      .hmap { |loc, count| [loc, [count, board[loc]]] }
      .hmap do |loc, (count, alive)|
    if rules[alive ? :s : :b].include?(count)
      alive = true
    else
      alive = false
    end
    [loc, alive]
  end.reject { |_, alive| !alive }
end

def draw_board(board)
  puts 'No living cells!' if board.empty?
  top_left, bot_right = board.boundaries
  top_left[1].upto(bot_right[1]) do |y|
    top_left[0].upto(bot_right[0]) do |x|
      print (board[[x, y]] ? 'X' : ' ')
    end
    puts
  end
end

generations = Integer(ARGV.shift)
rules = Hash[ARGV.shift.split('/').map(&:chars).map { |arr| [arr.shift.downcase.to_sym, arr.map(&:to_i)] }]
board = Hash[
    File.open(ARGV.shift) { |file| file.readlines.map { |line| line.chomp.chars } }
        .map.with_index { |row, y| row.map.with_index { |cell, x| [[x, y], cell != ' '] } }
        .flatten(1)
        .reject { |(_, data)| !data }
]
board.default = false

def pretty(list, conjunction)
  if list.length == 1
    list[0]
  elsif list.length == 2
    "#{list[0]} #{conjunction} #{list[1]}"
  else
    "#{list[0...-1].join(', ')}, #{conjunction} #{list[-1]}"
  end
end

puts 'Rules:'
puts "* Cells stay alive with #{pretty(rules[:s], 'or')} neighbors"
puts "* Cells are born with #{pretty(rules[:b], 'or')} neighbors"
puts '* In all other cases, cells die or stay dead.'
puts '***'

(0..generations).each do |round|
  puts "Generation #{round}:"
  draw_board(board)
  board = iterate(board, rules)
end

The syntax is as follows:

ruby game_of_life.rb <generations> <rules> <starting map>

where generations is a positive integer, rules is a string in the format of B3/S23; the B side represents when cells go from dead to alive and the S side represents when they stay alive between generations. starting map is the name of a file which contains Gen 0; spaces represent dead cells, any other (printable, non-newline) character represents a living one.

To simulate 50 generations of this pattern in Conway's Game of Life:

        t
bo       x
op     txt

save that pattern to a file called map.txt in the same directory as game_of_life.rb, then run this command:

ruby game_of_life.rb 50 B3/S23 map.txt

The output will be long, but should end with:

Generation 50:
XX                    
XX                    










                     X
                   X X
                    XX
\$\endgroup\$
  • \$\begingroup\$ hmap can use the implicit yield but I am not sure that would be an improvement \$\endgroup\$ – Caridorc Aug 10 '15 at 17:39
  • \$\begingroup\$ @Caridorc I prefer block.call because I'm more used to Java-esque programming (i.e. functional interfaces) and passing a block as a parameter, then calling it, is more comfortable than using a magic function. \$\endgroup\$ – Nic Hartley Aug 10 '15 at 17:42
  • \$\begingroup\$ Indeed, I did not post an answer as I think yield is not superior, just different \$\endgroup\$ – Caridorc Aug 10 '15 at 17:44
  • 1
    \$\begingroup\$ Your code is so coincise, I love it. 👍 \$\endgroup\$ – Caridorc Aug 10 '15 at 17:56
  • \$\begingroup\$ What you have could perhaps be described as a general 2-dimensional cellular automata \$\endgroup\$ – ankh-morpork Aug 10 '15 at 21:19
2
\$\begingroup\$

Wow, this sure is some beautiful code, posted by someone who's probably just as beautiful, despite being a fat fu.

I'm gonna kick off this review by saying that pretty is named pretty (hah, get it?) badly. I mean, come on -- what the heck does it do? Does it return a formatted string? If that's the case, how's it formatting? I recommend a name like format_array_as_sentence, or arr_as_sentence if you want it shorter. Heck, maybe even as_sentence, if you're feeling particularly ornery. Still, you were tired and sick of being so close but not done, so that's forgivable.

[[x_bounds[0], y_bounds[0]], [x_bounds[1], y_bounds[1]]] can be written better as x_bounds.zip(y_bounds), but since you didn't even know that function existed back when you wrote this, I'll let it slide. Wait, you did! Shame on you! Go sit in the corner.

Now that I'm out of the corner, I feel confident in asking: how in the ever-loving RNGesus does Array#neighbors work? I mean, it does, and it works beautifully, but how? Seriously, that's some top-level magic going on right there. Add a few comments with an example to step through what's happening, so when you have to debug because they removed product or renamed it you don't need to sit there going "whaaaaaaat" for two hours like you did last time you had to maintain code you stole from the internet

In iterate why do you do two hmaps? Why not just one that looks like this:

.hmap do |loc, count| 
  if rules[board[loc] ? :s : :b].include?(count)
    alive = true
  else
    alive = false
  end
  [loc, alive]
end

Wait, no, let's shorten that even more by not using a dumb extra variable where it's not needed:

.hmap { |loc, count| [loc, rules[board[loc] ? :s : :b].include?(count)] }

Now, that doesn't make much sense, so let's rename a few variables and change some symbols (which is gonna end up requiring some refactoring but that's okay):

.hmap { |loc, neighbor_count| [loc, alive_from[board[loc] ? :alive : :dead].include?(neighbor_count)] }

'course, that's gonna change a few things, like I said. First and foremost: You can't use that slick, shifty hack to get the symbols in place. You gotta do it the hard way. Well, it's not hard per se but shut up I'm doing your work for you so I get to use the words I want.

alive_from = Hash[%i[alive dead].zip(ARGV.shift.split('/').map(&:chars).map{|a|a.map &:to_i})]

Now, changing this means that the command-line syntax changes, too -- now, it's X/Y, instead of BX/SY. Much easier to remember.

Both iterate and draw_board are badly-placed, since everything else is object-oriented but those two bits are functional-style. Change one or the other, or make me cry by changing both.

print (board[[x, y]] ? 'X' : ' ') makes me just a little sad, since it means that you didn't realize that you should put the parens next to the method name because you can do that instead of relying on tricky spacing and expression calls.

Back to ragging on iterate: The check (board if board.empty?) at the looks like it should be useless, but damn if it doesn't speed up the empty rounds. Cute.

self.each_with_object(Hash.new 0) do |item, count|
  count[item] += 1
end

I don't think I'll ever understand your reasons for making this do/end instead of {/}, but change it anyway.

In draw_board, you should put a box around it, so that it's clear precisely where the boundaries are. Here's the updated code:

def draw_board(board)
  puts 'No living cells!' if board.empty?
  top_left, bot_right = board.boundaries
  puts "+#{'-'*(bot_right[1]-top_left[1])}+"
  top_left[1].upto(bot_right[1]) do |y|
    print '|'
    top_left[0].upto(bot_right[0]) do |x|
      print (board[[x, y]] ? 'X' : ' ')
    end
    puts '|'
  end
  puts "+#{'-'*(bot_right[1]-top_left[1])}+"
end

Bam! ASCIIArt quote for the week done.

\$\endgroup\$
  • \$\begingroup\$ OP here and wow this is a fantastic review! Thanks! Just saying, though, if you added the full code, it'd be a lot better. \$\endgroup\$ – Nic Hartley Nov 20 '15 at 3:49
  • \$\begingroup\$ @QPaysTaxes Once I finish the whole review, I'll post the edited code alongside it. \$\endgroup\$ – Nic Hartley Nov 20 '15 at 3:49
  • \$\begingroup\$ Things are getting a little solipsistic up in here :) \$\endgroup\$ – Flambino Nov 20 '15 at 11:16
  • \$\begingroup\$ @Flambino Just a little. It gets boring here at home. \$\endgroup\$ – Nic Hartley Nov 20 '15 at 22:39

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.