patternrubyMinor
Ruby Brainfuck interpreter
Viewed 0 times
brainfuckinterpreterruby
Problem
I recently wrote a simple Brainfuck interpreter in Ruby:
I am pretty bad at Ruby, can someone review this?
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 ""
exit 1
end
bf = BF.new
bf.run parse(File.read(ARGV[0]).chars)I am pretty bad at Ruby, can someone review this?
Solution
Global variables should always be avoided, so instead of
Instead of
I may be misunderstanding the scoping decisions taken here but it seems like
$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# 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.Code Snippets
ast.each {|e|
...
}ast.each do |e|
...
endif level != 0 then
throw Exception.new "Unmatched brackets"
endContext
StackExchange Code Review Q#142281, answer score: 5
Revisions (0)
No revisions yet.