eval() considered harmful

Before I started writing this post, I googled around a bit thinking that someone would have blogged about this before. I was surprised to only find articles about how great and powerful the JavaScript function eval() can be. I’m sure there are some uses for eval(), but I have never seen it being used where there wasn’t a cleaner, better way to do the same thing.

I found a fairly good article here that covers eval(), but I don’t think it emphasizes enough that there is probably a better way of doing things in most (almost all) cases.

The reason I started writing this entry was because I’m currently in the process of making a very large web-app that literally has tens of thousands of lines of javascript code Firefox compatible. Before I could replace something like document.all.([a-zA-Z_0-9#]*) with document.getElementById(”$1?) I had to fix up the hundreds of lines that unnecessarily uses eval(), because often document.all is used inside eval and my regular expression would have failed miserably. That’s my main concern - eval() is one of those things that makes it more difficult to refactor your code. I’m also fairly sure that most of the times it was used it had a little performance impact on the code (not like it really mattered - none of the javascript was in a tight loop) because I’m sure it is slower than the conventional methods for most operations. People also committed the sin of building up a string that represents the path to the element they want to use and then run eval() on the same string probably 20 times in the same function..

So, to summarize: eval() is usually not necessary, often adds unnecessary complexity to your code , usually is a sign of bad quality code in general and almost definitely makes it more difficult to maintain.

btw, the example mentioned at the link above:

var fieldList = new Array('field1','field2','field3','field4','field5','field6',
    'field7','field8');

var tempObj;

for (count=0;count<fieldList.length;count++)
{
    tempObj=EVAL("document.myForm." + fieldList[i]);
    if (tempObj.value.length==0)
    {
        alert(fieldList[i] + " requires a value");
    }
}

can be done without eval() like this:

var fieldList = new Array('field1','field2','field3','field4','field5','field6',
    'field7','field8');

var tempObj;

for (count=0;count<fieldList.length;count++) {
    tempObj=document.myForm.elements[fieldList[i]];
    if (tempObj.value.length==0)
        alert(fieldList[i] + " requires a value");
}

UPDATE:

I have noticed two good uses of eval: JSON and AJAX. When you serialize something as JSON on the server, the easiest and quickest way to turn that into javascript objects is to eval() the string. Also, if you return javascript code blocks inside AJAX responses, then you have to eval() it for it to execute. Use with caution..

Leave a Response

(will not be published)
(optional)
Remember Me