Alternative to using eval()

Question

I've heard a lot of rumblings about how "evil" or even "misunderstood" the eval function is, so I've decided to remove it from my code. The problem is I don't know what to replace it with.

Here's a quick rundown of my current code. I have a series of arrays (just 2 for the example below) declared at the beginning, and then based on a button click one of them gets loaded into a variable that is passed into a function.

Here's some basic HTML

<div class="button" data-name="button1">Button1</div>
<div class="button" data-name="button2">Button2</div>

and the JS (with jQuery)

var butName = null;
var eArray = null;
var button1Logo = ["..path/to/pic1.png","..path/to/pic2.png"];
var button2Logo = ["..path/to/pic3.png","..path/to/pic4.png"];

    $(".button").mouseup(function(){
            /*give a butName*/
            butName = $(this).attr("data-name");

            /*give the array from the button*/
            eArray = eval(butName + "Logo");
    });

Doing it this way assigns the array to the variable and not just a string that says "butnameLogo" which is why I used eval. But I'm looking to get away from that.

I know I can add a new attribute to the html and just retrieve that for the variable but I don't want to add more html when I can possibly do it with JS.

I've also tried making an object with strings loaded into it as seen in this answer: https://stackoverflow.com/a/16038097/1621380 but that resulted in just a string again, and not assigning a variable.

Wondering if you smart people have any better suggestions!

It's one of the most asked questions but usually with less details... — Denys Séguret, Nov 05 '13 at 20:43
The answer you get is going to be the same as the answer you found in that other question. — Blue Skies, Nov 05 '13 at 20:43
Whenever you encounter a series, make it an array (hashmap)! — ComFreek, Nov 05 '13 at 20:43
Just a quick tip you may be interested in - jQuery parses any data- attributes – so instead of `$(this).attr("data-name")` you could use `$(this).data("name")` (and set it using `$(this).data("name", "new value")`). It even converts it from string to number/object/array as appropriate. May help you one day. Docs: [(jQuery Docs)](http://api.jquery.com/data/#data-html5) — Shai, Nov 05 '13 at 21:39

score 8 · Accepted Answer · answered Nov 05 '13 at 20:45

Replace

var button1Logo = ["..path/to/pic1.png","..path/to/pic2.png"];
var button2Logo = ["..path/to/pic3.png","..path/to/pic4.png"];

with an object, where the keys are your button names:

var buttonLogos = {
    button1: ["..path/to/pic1.png","..path/to/pic2.png"],
    button2: ["..path/to/pic3.png","..path/to/pic4.png"]
};

Then instead of the eval you can simply do

eArray = buttonLogos[butName]

(or buttonLogos[butName + "Logo"] if you want to call the keys button1Logo and button2Logo, but I can't really see the point now that they are nicely contained within a buttonLogos object)

score 2 · Answer 2 · edited Nov 05 '13 at 21:03

2

The best option is to define an object which holds all our button paths:

var buttons = {
    "1": ["..path/to/pic1.png", "..path/to/pic2.png"],
    "2": ["..path/to/pic3.png", "..path/to/pic4.png"]
};
$(".button").mouseup(function(){
  /* give a butName */
  var butName = $(this).attr("data-name");

  /* give the array from the button */
  var eArray = buttons[butName];
});

If your variables reside in the global scope, you could use the bracket notation to access them:

eArray = window[butName + "Logo"];

Note that this solution is not recommended. The first code sample is much cleaner and more maintainable.

Imagine a situation where you would have to move all the code into a 'deeper' context (!= global context). Nothing would work anymore.

edited Nov 05 '13 at 21:03

ComFreek

29,044
18
104
156

answered Nov 05 '13 at 20:43

Denys Séguret

372,613
87
782
758

1

Not defining the variables in the global context would break this solution. – ComFreek Nov 05 '13 at 20:44
@dystroy Your original answer (with just the `window[butName + "Logo"]` part was (a) probably wrong and (b) a bad idea even if it was right! – lonesomeday Nov 05 '13 at 20:47
@dystroy I gave you an upvote now (and no, my comment wasn't ridiculous!). This answer is also CW, so don't care about downvotes. – ComFreek Nov 05 '13 at 20:48
@ComFreek You perfectly know the code was running in the global context and was easy to adjust anyway... – Denys Séguret Nov 05 '13 at 20:48
1

@dystroy "easy to adjust" -- are you sure? – John Dvorak Nov 05 '13 at 20:49
1

@dystroy The OP probably does run the code in the global context. This doesn't mean that we should provide solutions which depend on that fact. – ComFreek Nov 05 '13 at 20:51
@ComFreek 1) I explain. 2) I fix OP's problem. 3) a few seconds later I provide a better alternative. And that doesn't stop the mob from downvoting this CW answer instead of trying to make it better... SO is so disappointing sometimes... – Denys Séguret Nov 05 '13 at 20:52
@dystroy you still keep the bad solution around. That's why my downvote sticks. – John Dvorak Nov 05 '13 at 20:53
@dystroy Jan Dvorak is right. Moving the 'bad' solution to the end of the answer including a warning would be very good. Nevertheless, I gave you an upvote. I cannot do more. That's SO (, sadly). – ComFreek Nov 05 '13 at 20:55
it's still incorrect. It won't work outside the global scope (or outside a browser, by the way) – John Dvorak Nov 05 '13 at 20:55

score 2 · Answer 3 · answered Nov 05 '13 at 20:46

Use an object:

var butName = null;
var buttonsLogos = {
      button1: ["..path/to/pic1.png", "..path/to/pic2.png"],
      button2: ["..path/to/pic3.png", "..path/to/pic4.png"]
};

$(".button").mouseup(function(){
    /*give a butName*/
    butName = $(this).attr("data-name");

    /*give the array from the button*/
    eArray = buttonsLogos[butName];
});

score 2 · Answer 4 · answered Nov 05 '13 at 20:54

Consider making the data available as properties of an object, then you can control access to the object through scope and only need one (global?) variable for all such data.

If global scope is needed, then:

var dataObj = {
    button1Logo: ["..path/to/pic1.png","..path/to/pic2.png"],
    button2Logo: ["..path/to/pic3.png","..path/to/pic4.png"]
}

and later:

var eArray = dataObj[this.data-name + 'Logo'];

You may want to call the data object something more meaningful than dataObj though.

score 1 · Answer 5 · answered Nov 05 '13 at 20:46

You can do this very nicely with arrays and array indexes. You needn't find and use variable names at all. Even your data- attributes are unnecessary.

var eArray;
var buttonLogos = [
    ["..path/to/pic1.png","..path/to/pic2.png"],
    ["..path/to/pic3.png","..path/to/pic4.png"]
];

var buttons = $(".button").mouseup(function(){
    var idx = buttons.index(this);

    eArray = buttonLogos[idx];
});

The key line in this is buttons.index(this). This method call gets the position of the current element among all the elements matched by $(".button"). We then use this index to select the relevant element from the buttonLogos array.

score 1 · Answer 6 · answered Nov 05 '13 at 20:51

You're taking a very circuitous route by using eval here.

You'd be much better off doing something like this:

var paths = {
    button1: ["..path/to/pic1.png","..path/to/pic2.png"],
    button2: ["..path/to/pic3.png","..path/to/pic4.png"]
};

$(".button").mouseup(function(){
    /*give the array from the button*/
    eArray = paths[$(this).attr("data-name")];
});

eval should only be used if you need to execute code (usually from a 3rd party source), and even that is rare. If you ever find yourself saying "i should use eval here", there's almost definitely a better alternative, and you should try and find it.

"and you should try and find it." -- and thus he came here :-) — John Dvorak, Nov 05 '13 at 20:59

Alternative to using eval()

6 Answers6