From: David Masover <ninja@...>
Date: 2009-08-25T09:40:20+09:00
Subject: Re: Ruby eval

On Monday 24 August 2009 02:24:08 pm Brandon Larocque wrote:
> 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