1

I inherited maintenance on an app that uses eval() as a way to evaluate rules written in Ruby code in a rules engine. I know there are a lot of other ways to do it, but the code base so far is pretty big, and changing it to something else would be prohibitive time-wise at this point; so assume I'm stuck using eval() for the moment.

The rules as written typically call up some of the same objects from the database as each other, and the rules writer gave the variables in the rules the same names as each other. This is resulting in pages and pages of "already initialized constant" warnings in the console during development.

I'm wondering a couple things:

First, if feels like those are slowing down the execution of the program in the dev environment, and so I'm wondering if it's a big performance hit in the production environment, specifically, having those warnings pop, not eval() itself, which I know is a hit.

Second, is there any way to "namespace" the execution of each rules so that it's not defining its variables on the same scope as all the other evals in the request to avoid that warning popping all over the place? I know I could rewrite all the rules to use ||= syntax or to check if a name has already been defined, but there's quite a lot of them so I'd rather do it from the code that runs the eval()'s, if possible.

** update with example rule ** A question has a rule about when it's to be displayed to a user. For example, if the user has stated that they live in an apartment, another question might need to be shown to ask what size the apartment building is. So the second question's rule_text might look like:

UserLivesInApartment = Question.find_by_name "UserLivesInApartment"
UserLivesInApartment.answer_for(current_user)

The code that calls the eval ensures there's a current_user variable in scope prior to evaluating.

Paul
  • 35,689
  • 11
  • 93
  • 122
  • It would help to see some samples and example code. As is, this is a subjective question and we have to guess about your code which lessens the ability to help. – the Tin Man Dec 24 '11 at 22:08
  • That is just not correct ruby and should correctly generate a warning. Why would you assign a variable to a constant? I think you should refactor the code to make it either a class variable @user_lives_in_apartment or just a plain variable user_lives_in_apartment. You will be polluting the global namespace with results of active record queries otherwise. – sunkencity Dec 27 '11 at 13:28
  • This is an example of one of several hundred rules in the system that I inherited. what's the cleanest way to do the refactoring you're suggesting? – Paul Dec 27 '11 at 20:19
  • Sorry, I also don't understand how it's not correct ruby? I know it's a convention to use not use pascal casing, but how does that affect what we're talking about? – Paul Dec 30 '11 at 03:20
  • OK, after doing a bunch of trial and error (couldn't find an actual reference to this), I see you are correct and what I thought was convention is actually enforced. My mistake, and thanks on that. I guess the only way to go is to rewrite all those rules. – Paul Dec 30 '11 at 06:29

1 Answers1

1

uh, eval is not perhaps the most golden of standards. You could probably fire up a drb instance and run the stuff in that instead of eval, that way you would have at least some control of what is happening and not pollute your own namespace.

http://segment7.net/projects/ruby/drb/introduction.html

Edit: added another answer for running the code in the same process:

I don't know how your rule code looks, but it might be possible to wrap a module around it:

# create a module
module RuleEngineRun1;end
# run code in module
RuleEngineRun1.module_eval("class Foo;end")

# get results
#....

# cleanup
Object.send(:remove_const, :RuleEngineRun1)

You can also create an anonymous module with Module.new { #block to be module eval'd } if you need to run code in parallel.

In later rubies you can add -W0 to run your code without printing warnings, but doing so makes possible errors go unnoticed:

$ cat foo.rb 
FOO = :bar
FOO = :bar

$ ruby foo.rb 
foo.rb:2: warning: already initialized constant FOO

$ ruby -W0 foo.rb

You could also run your eval inside a Kernel.silence_warnings block, but might be devastating as well if you actually run into some real problems with the eval'd code, see Suppress Ruby warnings when running specs

Community
  • 1
  • 1
sunkencity
  • 3,482
  • 1
  • 22
  • 19
  • I know there's issues w/ eval(); as I said in the question, this is inherited code and that's how it's already done. That said, I would not mind switching to something else that makes more sense, but I fail to see how I could use drb to do what needs to be done (i.e. loading a script as a string from the DB and evaluating it). – Paul Dec 23 '11 at 21:42
  • The idea would be to shield off running the rules in a different process. The nice thing about running it like that would be to be able to restart it or wipe it. That way you could have a clean slate for a run of the rule engine. – sunkencity Dec 23 '11 at 22:19
  • There's some cool stuff in ruby 1.9.3 that can eval in a Binding. But I assume for a legacy app it would be hard to use 1.9.3 http://www.ruby-doc.org/core-1.9.3/Binding.html – sunkencity Dec 23 '11 at 22:20
  • When I try your sample code, I get NoMethodError on module_eval. When I instead call RuleEngine1.module_eval("code"), I get the same results as running regular eval(); i.e. it works, but I get the already defined constant errors. – Paul Dec 24 '11 at 07:06
  • Ok, messed up the code, yeah it should be like that, my bad. So what are you getting already defined error on, classes? You could check in Class.constants like "unless RuleEngineRun1.constants.include?(:Foo)" or if it's a variable that begins with a big letter (i.e. a constant) you could perhaps change it back to just a variable. There might be a way to turn the warnings off, but I suspect it would mean possibly breaking something else. – sunkencity Dec 24 '11 at 22:03
  • No, I've updated my question to give an example, but the typical pattern in a rule is to retrieve one or more other questions by name, check their answers by the current user, then decide whether this question needs to be shown or not. The already defined constants are the previously retrieved questions. – Paul Dec 27 '11 at 06:47