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
1 Answer 1
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:
- 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.
- A
hash.default=
call to set the default tofalse
(as "not here"), just for consistency withtrue
as "here". Note that this doesn't change anything about how the code actually runs, but it's more consistent to usetrue
/false
thantrue
/[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.
print
method hasn't (lexically) been defined before it's used. So it falls back to the built-inprint
instead, and just prints{[1, 0]=>true, [1, 2]=>true, [0, 2]=>true, ...
\$\endgroup\$