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?

Eli Foster · Accepted Answer · 2016-09-24 03:17:40Z

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.

Stack Exchange Network

Ruby Brainfuck interpreter

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Ruby Brainfuck interpreter

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions