22

I have a little bit of code that looks just like this:

function StrippedExample(i1, i2, i3, i4, i5, i6, i7, i8) {
    this.i = [];
    for (var i=1,j=0 ;i<9;i++) {
        var k = eval("i"+i);
        if (k > 0) {
            this.i[j++] = k;
        }
    }
}

FireBug profiler claims that second longest function is eval(), taking up to nearly 6% of the run time.

Everyone says eval is EVIL (as in bad) and slow (as I have found), but I can't really do anything else - the server simply pulls the data out the database and pushes to the browser.

What alternatives do I have? I could do the same as I am doing here on the server but that just shifts the burden higher up the chain. I can't change the database layout since everything hooks into those 8 variables and is a massive undertaking.

Community
  • 1
  • 1
graham.reeds
  • 16,230
  • 17
  • 74
  • 137

11 Answers11

18
function StrippedExample(i1, i2, i3, i4, i5, i6, i7, i8) {
    var args = [i1, i2, i3, i4, i5, i6, i7, i8]; // put values in an array
    this.i = [];
    for (var i=0,j=0 ;i<8;i++) { // now i goes from 0-7 also
        var k = args[i]; // get values out
        if (k > 0) {
            this.i[j++] = k;
        }
    }
}

The above code can be simplified further, I just made the minimal change to get rid of eval. You can get rid of j, for example:

function StrippedExample(i1, i2, i3, i4, i5, i6, i7, i8) {
    var args = [i1, i2, i3, i4, i5, i6, i7, i8];
    this.i = [];
    for (var i = 0; i < args.length; i++) {
        var k = args[i];
        if (k > 0) { this.i.push(k); }
    }
}

is equivalent. Or, to use the built-in arguments object (to avoid having your parameter list in two places):

function StrippedExample(i1, i2, i3, i4, i5, i6, i7, i8) {
    this.i = [];
    for (var i = 1; i < arguments.length; i++) {
        var k = arguments[i];
        if (k > 0) { this.i.push(k); }
    }
}

Even if you weren't filtering the list, you don't want to do something like this.i = arguments because arguments is not a real Array; it has a callee property that you don't need and is missing some array methods that you might need in i. As others have pointed out, if you want to quickly convert the arguments object into an array, you can do so with this expression:

Array.prototype.slice.call(arguments)

You could use that instead of the var args = [i1, i2 ... lines above.

benzado
  • 82,288
  • 22
  • 110
  • 138
  • 1
    This code is currently incorrect, as it's using 1-indexing while the array is 0-indexed. It's also definitely more complicated than it needs to be, as you could just replace `args` with the built in `arguments` array, and not define it yourself. – Brian Campbell Jan 08 '10 at 19:01
  • Thanks for catching that, Brian. I think I fixed it. – benzado Jan 08 '10 at 19:02
  • 1
    Now you've missed the `if (k > 0)` part from the original. – Brian Campbell Jan 08 '10 at 19:12
  • Thanks again. Now I think we're OK. What a weird piece of code. – benzado Jan 08 '10 at 19:48
  • 1
    Another reason to not do `this.i = arguments` is that `arguments` is also *missing* properties you might want in `i`, since it isn't an array. – Matthew Crumley Jan 08 '10 at 20:03
  • 1
    A better example would be `this.i = Array.prototype.slice.call(arguments)`. – Eli Grey Jan 09 '10 at 00:17
  • @Elijah: that's definitely more concise, but not very readable. Is it even correct? The first argument is supposed to be dropped. – benzado Jan 09 '10 at 00:33
  • The first argument isn't supposed to be dropped. Arguments are only supposed be dropped if they are equal to 0. – graham.reeds Jan 09 '10 at 01:13
  • I won't use the built-in `arguments` as there are other parameters being passed - this was a stripped down example. If I knew of the `arguments` keyword(?) then I would of realised it was relevant and then had extra parameters. – graham.reeds Jan 09 '10 at 01:21
  • Sorry I got the `(k > 0)` part wrong. Just made a (hopefully) final edit. Glad it was helpful all the same. – benzado Jan 11 '10 at 02:30
12

Eval alternative:

exp = '1 + 1'
x = Function('return ' + exp)()
console.log(x)
Let Me Tink About It
  • 15,156
  • 21
  • 98
  • 207
Arthur Ronconi
  • 2,290
  • 25
  • 25
  • 1
    +1 For suggesting an alternative to eval(). Unfortunately it is rejected by CSP (default-src 'self') just like eval in Firefox. – Code4R7 Dec 13 '21 at 20:55
8

You are simply making an array from your function 8 arguments, removing the ones that are less than or equal to zero.

The following code is equivalent, and it will work for any arbitrary number of arguments:

function StrippedExample() {
  var args = [];

  for (var i = 0; i < arguments.length; i++) {
    if (arguments[i] > 0) {
      args.push(arguments[i]);
    }
  }
  //...
}
Christian C. Salvadó
  • 807,428
  • 183
  • 922
  • 838
5
  1. Call the function with one argument — an Array
  2. Use the arguments object
Quentin
  • 914,110
  • 126
  • 1,211
  • 1,335
4

One alternative to to pass an array to your function, instead of individual arguments:

StrippedExample([3, 1, 4, 1, 5, 9, 2, 6])

Then your code would be:

function StrippedExample(inArray) {
    this.i = [];
    for (var i=0,j=0 ;i<inArray.length;i++) {
        var k = inArray[i];
        if (k > 0) {
            this.i[j++] = k;
        }
    }
}

If you really need to pass in separate arguments, you can access them using your arguments array, which is an object that acts like an array (though it's not really; not all Array methods work on it) that exposes all arguments that have been passed in to your function; they do not even need to be declared in this case, but it's good form to include a comment indicating what sorts of arguments you are expecting for users of your code:

function StrippedExample(/*i1, i2, i3, i4, i5, i6, i7, i8*/) {
    this.i = [];
    for (var i=0,j=0 ;i<arguments.length;i++) {
        var k = arguments[i];
        if (k > 0) {
            this.i[j++] = k;
        }
    }
}

If you're guaranteed to only have 8 elements, then you could use 8 in place of inArray.length or arguments.length; I decided to use the more general version in my examples in case that was helpful to you.

Brian Campbell
  • 322,767
  • 57
  • 360
  • 340
1

This code should be made to use the arguments array that every Javascript function has access to.

It's not that eval is evil (it's in Lisp, so it must be good) it's simply a sign of a hack - you need something to work and you forced it. It screams out to me "The author gave up on good programming design and just found something that worked".

Frank Krueger
  • 69,552
  • 46
  • 163
  • 208
  • I'm not au fait with all the ins and outs of Javascript. I can hack around to get things done, but as demonstrated here, it's not always the best way. – graham.reeds Jan 09 '10 at 01:17
  • So one of these complicated workaround aren't hacks, but this convenient, one line function is the hack? – johny why Nov 16 '22 at 02:41
1
function StrippedExample() {

    this.i = [];
    for (var i=1,j=0 ;i<arguments.length;i++) {
        var k = arguments[i];
        if (k > 0) {
            this.i[j++] = k;
        }
    }
}
Gabriele Petrioli
  • 191,379
  • 34
  • 261
  • 317
1
  1. Short answer:
StrippedExample=(...a)=>a.filter(i=>i>0);

no need use eval for work with arguments at all.

  1. Initial code and most proposed solutions doesn't return result by traditional way.
in4
  • 11
  • 2
0

Given that there is a fixed amount of variables, you can build an array of them manually and loop through it. But if you have a variable amount of arguments, one way to get the variables passed to the function as an array is:

var args = Array.prototype.slice.call(arguments.callee.caller.arguments);

And your function would look like this:

function StrippedExample() {
    var args = Array.prototype.slice.call(arguments.callee.caller.arguments);
    for(var i in args) {
        if (args[i] > 0) {
            this.i[j++] = args[i];
        }
    }
}
Tatu Ulmanen
  • 123,288
  • 34
  • 187
  • 185
0

definitely try this as a drop in replacement. I was looking for this answer and came to this post. I read the developer doc and this worked as a direct replacement in my application

var func1 = "stringtxt" + object  + "stringtxt";


instead of --->    eval(func1);    --> use -->   Function(func1)(); 

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval#Never_use_eval!

Code Lღver
  • 15,573
  • 16
  • 56
  • 75
  • Why is this downvoted? It's the suggested alternative here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval – Justin Putney Feb 03 '22 at 21:39
0

You can also run string expressions with setTimeout. It works the same as the Function object.

let a=10;
window.a=100;
window.b=1;
setTimeout("let c=1000;console.log(a,b,c)");//10,1,1000
a.gulcan
  • 171
  • 3
  • 9