Correct use of eval in JavaScript

Question

I want to load a different audio file clicking on different texts in a web page. I use the following jQuery function:

var audio = document.createElement('audio');      
$(".text_sample").on("click", function() {
  audio.src = eval(this.id);
  audio.play();
});

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<a class="text_sample" id="a_music">music</a><br />
<a class="text_sample" id="a_weather">weather</a>

Do you think is correct to use eval in this context? Are better way to do it?

Huh? But what does `a_music` evaluate to? This code should just throw an "Uncaught ReferenceError" — Joseph Marikle, Oct 24 '17 at 14:53
@JosephMarikle actually, if you have an element on the page with an id, browsers (maybe not all of them?) will add a variable to the window object. So window.a_music will be defined. Still not the right approach, but it won't throw an error unless you're in strict mode — Nicholas Tower, Oct 24 '17 at 14:55
@NicholasTower Ah! Yes, you are right. I completely forgot that was a thing. — Joseph Marikle, Oct 24 '17 at 14:58
Sorry, I closed as dup, but realized you asked for getting a variable while the other question was for setting a variable. Anyway, you might find interesting information from this question: https://stackoverflow.com/questions/26019658/get-elements-id-and-set-it-as-variable — Karl-André Gagnon, Oct 24 '17 at 15:08

score 1 · Accepted Answer · answered Oct 24 '17 at 14:53

1

Use the following if it is a global variable.

audio.src = window[this.id];

answered Oct 24 '17 at 14:53

rrk

15,677
4
29
45

Thanks for the answer, can you explain me why this is better? – Gusepo Oct 24 '17 at 14:58
1

The reason is the `eval` is considered as a bad coding practice by most experts and it is not really required in you case. `eval(this.id)` suggested that there was variables with names `a_music` and `a_weather`. All the global variables can be found inside the `window` object. So instead of `eval` we can simply use a `structure` reference for getting value. – rrk Oct 24 '17 at 15:52

score 0 · Answer 2 · answered Oct 24 '17 at 14:59

So it seems that you have some variables that correspond to the IDs of the element and you want to use the element ID to get the value of the variable.

While eval works, I wouldn't do it. There's not much of a security issue, as any eval'd code would only affect the user's own machine, but a cleaner approach would be to use data- attributes instead.

  <a class="text_sample" id="a_music" data-src="/foo.wav">music</a><br />
  <a class="text_sample" id="a_weather" data-src="/bar.wav">weather</a>

Then there's no need to rely on local variables at all. Instead use the dataset object on the element to get the src.

  var audio = document.createElement('audio');      
  $(".text_sample").on("click", function() {
      audio.src = this.dataset.src;
      audio.play();
  });

This also avoids killing optimizations as eval may do.

Correct use of eval in JavaScript

2 Answers2