Ruby eval

[Brandon Larocque]

I am trying to learn Ruby as a first language, ( I tended to go back and
forth over languages in my studies, but I plan, now, to focus on Ruby ),
and came across a confusing problem. My brother likes to play Mafia Wars
on facebook and asked me to come up with something simple. So I put a
little something together, and it involved the eval function.

From what I've read, recently, people tend to look down on the eval
function? Why is this? I have a few classes that have multiple names, so
I decided to use it to work through the problem I was having ( I didn't
want to write a bunch of loops ). For example :

( $jobarea == "hi" or $jobarea == "es" ) ? (loop = 9) : ( ($jobarea ==
"ca") ? (loop = 8) : (loop = 7) )
expcost = [] ; exppay = [] ; pay = [] ; name = []
for i in (1..loop) do
eval("expcost.push $#{$jobarea}_job#{i}.expcost")
eval("exppay.push $#{$jobarea}_job#{i}.exppay")
eval("pay.push $#{$jobarea}_job#{i}.pay")
eval("name.push $#{$jobarea}_job#{i}.name")
end

Is there any way that I might be able to do this without eval? Or is it
alright the way I have done it?

Regards,
Brandon LaRocque

 

[kallen]

It's not so much that eval is looked down on, it's very useful under
the right circumstances, it's just that it's like putting in a nail
with a sledgehammer, there's no need for it.

From what I can tell from your code you have a bunch of variables
named like es_job1, es_job2, etc. Rather then encode information like
jobarea and the number in the variable name, use a data structure to
do it. For instance you could use a hash of arrays like so:

jobs = { "hi" => [Job.new, Job.new, Job.new],
"es" => [Job.new, Job.new, Job.new] }

Then most of your code becomes moot. You don't need the loop variable
because you can loop across the hash and the arrays with each or other
methods like it. For example to get an array of all the expcosts like
you are doing could be done like so:

expcost = jobs.values.collect{|v| v.collect{|job|
job.expcost}}.flatten

Or even better, you can avoid all this by keeping all jobs in one
array and just using collect on the array.

Basically what you should take away from this is 1) that eval is
rarely the right choice and 2) that there are data structures like
arrays and hashes in ruby for precisely these kinds of things, use
them!

Also, for the first line you should be using if or case instead of the
ternary operator (?).

 

[David Masover]

From what I've read, recently, people tend to look down on the eval
function? Why is this?

It has a few specific uses -- irb wouldn't work without it, for example -- but
in general, there is a better way to do it, whatever it is.

Personally, my biggest beef with it is that I simply find it less readable,
because, among other things, my text editor doesn't highlight Ruby syntax
inside strings, even if those strings are being fed into eval.

But, let's take a contrived example. Suppose we didn't have attr_accessor:

%w(mine yours henry's).each do |name|
eval "
def #{name}
@#{name}
end
def #{name}= value
@#{name} = value
end
"
end

Can you spot the problem with that code?

Now, imagine that list of names came from elsewhere, or worse, from user
input. You could easily end up with a SQL injection problem, or something
worse. There's just a whole class of weird, hard-to-debug problems that won't
happen unless you use eval.

Finally, people dislike eval for the same reason they dislike goto -- in
general, if you're abusing eval, it's a sign that you should refactor your
code, and eval will just lead you to a spaghetti-code mess.

I have a few classes that have multiple names

I actually have no idea what you mean by this. Your example code shows exactly
zero indication of "classes that have multiple names".

( $jobarea == "hi" or $jobarea == "es" ) ? (loop = 9) : ( ($jobarea ==
"ca") ? (loop = 8) : (loop = 7) )
expcost = [] ; exppay = [] ; pay = [] ; name = []
for i in (1..loop) do
eval("expcost.push $#{$jobarea}_job#{i}.expcost")
eval("exppay.push $#{$jobarea}_job#{i}.exppay")
eval("pay.push $#{$jobarea}_job#{i}.pay")
eval("name.push $#{$jobarea}_job#{i}.name")
end

First of all, you're using global variables. Giant red flag there, just like
eval. Is there a reason those aren't either constants or local variables?

So, first, I'd restructure it a bit:

Jobs = {
'hi' => [Job.new(...), Job.new(...), ...],
'es' => ...
}

You get the idea -- put things in arrays or hashes, not global variables.
Glancing over at kallen's mail, it seems he came up with the exact same
thing...

Aside from making it easier to loop through, it makes it much easier to add
other jobs later on. For example, you could do:

Jobs['hi'].push Job.new(...)

You don't have to change the loop or anything like that -- the number of jobs
in that area can be found with:

Jobs['hi'].length

Now, those "bunch of loops" you didn't want to write:

results = Hash.new{|k,v| k[v]=[]}
Jobs.each_value do |jobs_in_area|
jobs_in_area.each do |job|
results['expcost'].push job.expcost
results['exppay'].push job.exppay
results['pay'].push job.pay
results['name'].push job.name
end
end

Even that is a bit repetitive. I'd probably turn that inner loop into
something like this:

%w(expcost exppay pay name).each do |field|
results[field].push job.send(field)
end

 

[Mason Kelsey]

[Note: parts of this message were removed to make it a legal post.]

Others can correct me if I'm wrong, as I'm also new to Ruby. (My 127th
language.) But eval , which also exist in several other languages, is used
when you need to change a COMMAND dynamically, after the program starts
running and is beyond the reach of the programmer. Since that is a fairly
complex situation and beyond the need for most programmers, eval should
normally not be used in production systems. I suspect that eval would be
caught in any production walkthrough by other professional programmers as a
potential security threat, since it would be easy to obscure what the code
is actually doing. For example, it could be used to create a trap door. In
that case the programmer would have to rewrite the code without using eval
to comply. So it is a waste of valuable time in most production systems
development or maintenance to use that command. I assume that eval in Ruby
is the same thing. Use eval for experimental or academic stuff, for example
in artificial intelligence.

No Sam

 

[Brian Candler]

( $jobarea == "hi" or $jobarea == "es" ) ? (loop = 9) : ( ($jobarea ==
"ca") ? (loop = 8) : (loop = 7) )
expcost = [] ; exppay = [] ; pay = [] ; name = []
for i in (1..loop) do
eval("expcost.push $#{$jobarea}_job#{i}.expcost")
eval("exppay.push $#{$jobarea}_job#{i}.exppay")
eval("pay.push $#{$jobarea}_job#{i}.pay")
eval("name.push $#{$jobarea}_job#{i}.name")
end

Is there any way that I might be able to do this without eval? Or is it
alright the way I have done it?

There are two big problems with string eval: it's very slow (especially
if used in a loop, as you have done), as it recompiles the string every
time it runs; and it may leave your program open to security exploits if
any of the data you're eval'ing comes from an external source. So here
are some alternatives.

Look at 'send' for dynamic method dispatch:

x = "hello"
m = "upcase" # or m = :upcase
x.send(m)

Look at 'const_get' for dynamic class/constant access:

klass = "String"
obj = Object.const_get(klass).new

Look at 'instance_variable_set' / 'instance_variable_get' for dynamic
access to instance variables:

var = foo
instance_variable_set(var, 123)
instance_variable_get(var)

I didn't find an equivalent to these methods for global variable access,
but that's a pretty horrible way to work anyway. Others have shown that
a Hash is the right data structure to use in your code.

Look at instance_eval if you just want to run fixed code on a dynamic
receiver.

s = "hello"
s.instance_eval { upcase }

 

[Brandon Larocque]

Well, I'm working on a rewrite, thanks to you guys

But, for those interested in what I'm doing in full, here's the code in
full :

Main file : http://pastebin.com/m5538752
Data file : http://pastebin.com/m48a54855