JavaScript eval() syntax

Question

I have an old script for changing the font size of a website by adding a class name to the body tag, and part of its functionality is to remove the existing class before changing it to one of the other settings. The problem is there's an eval line that removes any instance of the words "large," "medium," or "small" from other classes on the body tag which act as unique identifiers of the page, which interferes with other scripts I'm using. How can I change the eval line in the following code so that it only matches whole words?

/* Override CSS with global font size selected by user */
function changeFontSize(size) {
var oldClasses, currentClass;

/*sets key words to be eliminated*/
oldClasses = eval("/large|medium|small/ig");

/*gets the current class names*/
currentClass = document.body.className;

/*eliminates key words from string, then adds new size*/
document.body.className = currentClass.replace(oldClasses, "") + " " + size;
}

... Why is there eval here in the first place?!? oldClasses = /large|medium|small/ig is the same, except not evil. If you need to customise the string, oldClasses = new RegExp(['large', 'medium', 'small'].join('|'), 'ig'). (Not answering the question here, just expressing incredulity.) — Amadan
– Amadan, Commented Jul 19, 2017 at 23:09
"How can I change the following line so that it only searches for whole words?" The Question is not clear. Where does a "search" occur? — guest271314
– guest271314, Commented Jul 19, 2017 at 23:28
I'm not really familiar with eval, so I can't tell you why it was necessary. I assume from the context that its purpose is to list the terms that the script looks for among the classes assigned to the body tag. — Tom
– Tom, Commented Jul 19, 2017 at 23:48

Samuel Toh · Accepted Answer · 2017-07-20 03:49:00Z

3

There is no good reason to use the eval(...) operation here. Like many mentioned here, eval is bad practice.

Read more about it here: what does eval do and why its evil?

Doing eval("/large|medium|small/ig") is the same as var pattern = /large|medium|small/ig/. The former would evaluate the string to figure out what that expression means before deriving it as a regular expression literal. Whereas the latter is a straight forward declaration, essentially it is more efficient as you are skipping the evaluation steps.

Since the font pattern is static (doesn't change), it is always better to declare it as a regular expression object and keep using it.

Example:

var FONT_SIZE_NAMES_PATTERN = new RegExp(/\b(large|medium|small)\b/ig);

function changeFontSize(size) {
    var oldClasses, currentClass;

    /*gets the current class names*/
    currentClass = "large";

    /*eliminates key words from string, then adds new size*/
    // document.body.className = currentClass.replace(FONT_SIZE_NAMES_PATTERN, "") + " " + size;
    console.log("New class name = " + currentClass.replace(FONT_SIZE_NAMES_PATTERN, "") + " " + size);
}

changeFontSize("VERY LARGE");

edited Jul 20, 2017 at 3:49

answered Jul 20, 2017 at 3:43

Samuel Toh

19.5k3 gold badges29 silver badges40 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Tom Over a year ago

I had to tweak it a bit, but that worked very well. Thank you!

Collectives™ on Stack Overflow

JavaScript eval() syntax

1 Answer 1

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related