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.
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
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
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
ast.each do |e|
Every instance of
then in your code is unnecessary.
ARGV == nil call, firstly it can be optimized to
ARGV.nil?. After that, it might be better to use the boolean-y value of
if ARGV == nil ->
if ARGV.nil? ->
if level != 0 then
throw Exception.new "Unmatched brackets"
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
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