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.