0

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;
}
Tom
  • 275
  • 7
  • 20
  • 1
    ... 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 Jul 19 '17 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 Jul 19 '17 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 Jul 19 '17 at 23:48

1 Answers1

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");
Samuel Toh
  • 18,006
  • 3
  • 24
  • 39