1
\$\begingroup\$

I wrote a Langton's Ant cellular automaton simulator in Ruby using object oriented principles. This code works. It requires the drawille library.

I would like to get feedback on what I could do better here, from optimizing the display to cleaning up code. I'm also wondering what I could improve in code quality and style.

#!/usr/bin/env ruby
require 'drawille'
# Langton's Ant
# Draws in terminal
class Direction
 @dir = 0
 def initialize(direction)
 @dir = {up: 0, right: 1, down: 2, left: 3}[direction] || 0;
 end
 def transform_vector
 case @dir
 when 0
 return {x: 0, y: 1}
 when 1
 return {x: 1, y: 0}
 when 2
 return {x: 0, y: -1}
 when 3
 return {x: -1, y: 0}
 end
 end
 def turn_cw
 @dir = (@dir + 1) % 4
 end
 def turn_ccw
 @dir = (@dir + 3) % 4
 end
end
module State
 OFF = 0
 ON = 1
end
class Tile
 @state = State::OFF
 def initialize(state)
 @state = state
 end
 def state
 return @state
 end
 def flip
 if @state == State::OFF
 return @state = State::ON
 else
 return @state = State::OFF
 end
 end
 def bool
 return (@state == State::ON ? true : false)
 end
end
class Screen
 @screen = [];
 @canvas = Drawille::Canvas.new
 @x = 0
 @y = 0
 def initialize(x, y)
 @screen = Array.new(y) { Array.new(x) { Tile.new State::OFF } }
 @canvas = Drawille::Canvas.new
 @x = x
 @y = y
 end
 def inspect
 string = "";
 @screen.each do |i|
 i.each do |j|
 string << (j.bool ? '██' : ' ')
 end
 string << ?\n
 end
 return string
 end
 def screen
 return @screen
 end
 def flip(x, y)
 return @screen[y][x].flip
 end
 def print
 @screen.each_with_index do |row, y|
 row.each_with_index do |cell, x|
 if cell.state == State::ON
 @canvas.set(x, y)
 else
 @canvas.unset(x, y)
 end
 end
 end
 puts @canvas.frame
 end
 def x
 @x
 end
 def y
 @y
 end
end
class Ant
 @x = 0
 @y = 0
 @direction = :up
 @screen = nil
 def initialize(x, y, direction, screen)
 @x, @y, @direction, @screen = x, y, direction, screen
 end
 def current_cell
 @screen.screen[@y][@x]
 end
 def step
 if current_cell.state == State::ON
 @direction.turn_cw
 else
 @direction.turn_ccw
 end
 @screen.flip(@x, @y)
 @x = (@x + @direction.transform_vector[:x]) % @screen.x
 @y = (@y + @direction.transform_vector[:y]) % @screen.y
 end
end
def main
 screen = Screen.new(160, 96);
 ant = Ant.new(79, 47, Direction.new(:down), screen);
 loop do
 screen.print
 ant.step
 end
end
main
asked Dec 8, 2017 at 21:46
\$\endgroup\$
2
  • 1
    \$\begingroup\$ The Tile class and its collaborators go to a lot of effort to make 0 and 1 act like booleans. Why not just use false and true? \$\endgroup\$ Commented Dec 13, 2017 at 5:14
  • \$\begingroup\$ @ReinHenrichs Langton's Ant has variants that are more interesting - they require a tile having multiple types, so I wrote the class for extensibility purposes. \$\endgroup\$ Commented Dec 13, 2017 at 6:23

1 Answer 1

2
\$\begingroup\$

The code looks pretty clean. On first sight I notice some odd Ruby code.

Non-idiomatic Ruby

class Tile
 @state = State::OFF
 def initialize(state)
 @state = state
 end
 def state
 return @state
 end
 def flip
 if @state == State::OFF
 return @state = State::ON
 else
 return @state = State::OFF
 end
 end
 def bool
 return (@state == State::ON ? true : false)
 end
end

In the code above you first set @state and then inside initialize you set @state again. These two are not the same variable. Do not define instance variables outside class methods, unless you want to do some meta-programming with them.

You define method state for accessing the @state variable. As this is such a common pattern, Ruby provides a helper for generating such simpler getter methods:

class Tile
 attr_accessor :state

But all the places where state method is used, could actually use the bool method instead. This should make one realize that this whole Tile class could be completely eliminated and replaced with a plain boolean value instead.

Other smells

  • Screen class has @screen instance variable. It's not referring to itself, so a better name would be in order.
  • It's also exposing it directly to the outside world through screen method. Better to have a method like get(x,y) that would encapsulate the access to the coordinates array.
  • Screen has an inspect method that's unused. For debugging?
  • Screen has x and y fields, I think these are better called width and height.
  • Screen has a print method. I would instead recommend having a to_s method instead, so the Screen would not have a knowledge about printing itself, only of how to convert itself into string. One can then simply call puts screen to print it to console (the to_s method will be call automatically when casting to string).
  • Inside Ant.initialize all the variables are initialize on the same line, which I find confusing.
answered Dec 8, 2017 at 22:40
\$\endgroup\$
1
  • \$\begingroup\$ I have the Tile class for future extension (in case I want to implement some Langton's Ant variants). The bool method is one that I forgot to remove. I used Screen#inspect for output before I switched to drawille. Thanks for the tips on attr_accessor, field names, instance variables, to_s, etc. I find the one line initialization cleaner and easier to read, but if it's confusing for most people I'll change it. \$\endgroup\$ Commented Dec 9, 2017 at 1:00

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.