Ruby Brainfuck interpreter

Question

I recently wrote a simple Brainfuck interpreter in Ruby:

require "io/console"
$VALID = "+-<>.,"
def parse(code, level=0)
 ast = []
 while !code.empty? do
 c = code.shift
 if c == "]" then
 if level == 0 then
 throw Exception.new "Unmatched brackets"
 else
 return ast
 end
 end
 if c == "[" then
 ast.push parse(code, level+1)
 elsif $VALID.include? c then
 ast.push c
 end
 end
 if level != 0 then
 throw Exception.new "Unmatched brackets"
 end
 ast
end
class BF
 def initialize
 @tape = Array.new 3e4, 0
 @ptr = 3e4.to_i / 2
 end
 def run(ast)
 ast.each {|e|
 if e.kind_of? Array then
 while @tape[@ptr] != 0 do
 self.run e
 end
 else
 case e
 when "+"
 @tape[@ptr] += 1
 when "-"
 @tape[@ptr] -= 1
 when ">"
 @ptr += 1
 when "<"
 @ptr -= 1
 when "."
 STDOUT.write @tape[@ptr].chr
 when ","
 @tape[@ptr] = STDIN.getch.ord
 end
 @tape[@ptr] &= 0xFF
 end
 }
 end
end
if ARGV[0] == nil then
 puts "Simple BrainFuck interpreter"
 puts "Usage: #{0ドル} <file>"
 exit 1
end
bf = BF.new
bf.run parse(File.read(ARGV[0]).chars)

I am pretty bad at Ruby, can someone review this?

Answer 1

Global variables should always be avoided, so instead of $VALID it should simply be VALID. The entire Ruby ecosystem does not need access to that variable and it might overwrite some important variable somewhere.

Instead of Exception you should be using StandardError. Rubyists do not rescue Exceptions ever really because it catches every single possible exception or error, so if someone (including you) wants to rescue from parse, it will be confusing and non idiomatic. Exception is the top level class for all errors, including syntax errors, interruption errors, and memory errors. I would recommend either doing the simple fail "Error", or take a more object oriented approach (which will improve potential rescues) and create a new error object, perhaps UnmatchedBracketsError which inherits StandardError. This also means that when you decide to change the message, you only need to change it in the UnmatchedBracketsError class rather than in both of the identical throw calls.

I may be misunderstanding the scoping decisions taken here but it seems like parse should be a method in the BF class. It can be "static" by doing def self.parse, but it just seems weird to have it in the main namespace. Moving that would also mean it would make sense to move VALID out of the main namespace and into BF.

Array#<< is faster than Array#push so I recommend using that.

Now for the stylistic things.

Typically, multiline blocks are defined using do end syntax rather than {}. So

ast.each {|e|
 ...
}

becomes

ast.each do |e|
 ...
end

Every instance of then in your code is unnecessary.

For that ARGV[0] == nil call, firstly it can be optimized to ARGV[0].nil?. After that, it might be better to use the boolean-y value of ARGV[0]. So if ARGV[0] == nil -> if ARGV[0].nil? -> unless ARGV[0].

if level != 0 then
 throw Exception.new "Unmatched brackets"
end

can be shortened to throw Exception.new("Unmatched brackets") if level != 0.

Both of your while loops are using negated conditionals, which means they can be idiomatically changed to until loops. while !empty? can become until empty? and while a != 0 can become until a == 0.

Lastly, I'm pretty sure self can be omitted in the run call.