7
\$\begingroup\$

We ran a Code Retreat day at Hacker Paradise this Saturday, and I was pretty happy with my eventual solution. It's written in Ruby, but would work reasonably well in JS/CS and is (hopefully) reasonably readable across languages.

module Enumerable
  def bag
    # [1,1,1,1,2,2,3] => {1 => 4, 2 => 2, 3 => 1}
    # similar to python's Counter
    Hash[group_by { |x| x }.map {|k, v| [k, v.count] }]
  end
end

def neighbors_of(location)
  x_range = (location.first-1).upto(location.first+1).to_a
  y_range = (location.last-1).upto(location.last+1).to_a
  x_range.product(y_range) - [location]
end

def will_be_alive(was_alive, neighbors_count)
  if was_alive
    [2,3].include? neighbors_count
  else
    neighbors_count == 3
  end
end

def iterate(old_grid)
  old_grid.flat_map do |loc, _|
    neighbors_of(loc)
  end.bag.select do |loc, alive_near_me|
    will_be_alive(old_grid[loc], alive_near_me)
  end
end

def print(grid)
  system('clear')
  min_x, max_x = grid.keys.map(&:first).minmax
  min_y, max_y = grid.keys.map(&:last).minmax

  result = min_y.upto(max_y).map do |y|
     min_x.upto(max_x).map do |x|
      grid[[x,y]] ? 'X' : ' '
    end.to_a.join("")
  end.to_a.join("\n")

  puts result
end

def new_grid()
  glider = [[1,0], [1, 2], [0,2], [2,2], [2,1]]
  box = [[-3,3], [-3,4], [-4,3], [-4,4]]
  alive_start = glider + box
  Hash[alive_start.map { |k| [k, true] }]
end

grid = new_grid()

until grid.empty? do
  print(grid)
  grid = iterate(grid)
  sleep 1
end
\$\endgroup\$
  • \$\begingroup\$ Your last edit actually broke the code; it now outputs a raw hash because your print method hasn't (lexically) been defined before it's used. So it falls back to the built-in print instead, and just prints {[1, 0]=>true, [1, 2]=>true, [0, 2]=>true, ... \$\endgroup\$ – Flambino Nov 10 '14 at 4:23
5
\$\begingroup\$

I'm using this as my style guide here.

Don't use parentheses when there aren't parameters. In this case, it applies to new_grid's definition and uses -- rather than new_grid(), it should be just new_grid.

In new_grid, I'd recommend adding two things:

  1. A comment just before the last line to explain why you're not using nested (i.e. 2D) arrays. I suspect it's for memory reasons, but you may well have your own reasons for this somewhat counterintuitive choice.
  2. A hash.default= call to set the default to false (as "not here"), just for consistency with true as "here". Note that this doesn't change anything about how the code actually runs, but it's more consistent to use true/false than true/[falsey value].

In your actual run loop, you should separate the iterations by something, so they don't blend together. In the console (which is how I ran this), this is what it looks like:

A jumbled mess of <code>X</code> and <code></code>.

It's like this because system('clear') doesn't work on Windows. If you wanted to support Windows, you'd use system('cls') instead. (Before we go arguing about which OS is best, it doesn't matter. This is a code review. I'm finding bugs.)

However, since trying to make console commands work on every system is hard, I'd recommend simply putting some divider between the boards. For example, immediately after your current last line, you could put this:

puts '-' * (max_x - min_x + 1)

which will change the output to something more like this (I forgot where I originally took the picture, so it's not exactly the same area):

Roughly the same jumbled mess, this time with dividers

Or, alternatively, combine them! Try both clear and cls, and if neither succeed, puts a divider. Just get rid of system('clear') and toss this if before puts result to get that:

if system('clear')
  #Empty intentionally
elsif system('cls')
  #Empty intentionally
else
  puts '-' * (max_x - min_x + 1)
end

It works on my Bash emulator, my Ubuntu server, my Windows 8 laptop, and with some dark magicks I confirmed that if it can't run any of the commands it successfully outputs the dividers.

Testing with RubyMine confirms that RubyMine is weird. cls returns true, but does nothing. This is a (now reported) bug with RubyMine.

Now, while we're nitpicking print, have another: Rather than joining the lines with \n then outputting all at once, which may cause some errors with things that try to autoindent code (cough my stuff that puts spaces in front so it's easier to take pictures cough), replace it with .each { |line| puts line }.

Note: If you use both this and the last tip, you'll have to move the if block(s) up above it.

You've got a couple of magic numbers in will_be_alive, but it's Conway's Game of Life and the function is called will_be_alive, so it's pretty obvious what they are, so that's not that big a deal.

Aside from that, it looks good!

I like how you commented in bag to explain what it does, since it took me a good four minutes to get what product does.

The actual algorithm is good, though it took me a minute to figure out how iterate works when it seems like it should just give 8 every time, at least at first.

I quite like how you pass the grid as an argument to each function, rather than using a global variable or the like.


As I can tell, the majority of the code is good! Well done.

\$\endgroup\$

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.