Anti-Pattern


Anti-pattern is a set of patterns many developers habitually use, but is rejected for the sake of performance, debugging, maintenance, and readability. This document explains each anti-pattern with examples and provide suggestions for improvement.

Table of Contents

  • Include the <script> tag at the bottom of the document.
  • Do not use the direct URL when using external resources.
  • Do not use global variables.
  • Always declare import statements at the top.
  • Do not use variables without declaration.
  • Do not use constructors when creating arrays and objects.
  • Do not use the == to check for equality.
  • Do not omit the curly braces ({…}).
  • Do not omit the second parameter, base, of parseInt.
  • Do not omit the break clauses in switch statements.
  • Do not use for-in to iterate through an array.
  • Do not use delete to delete an element from an array.
  • Do not process any task unrelated to the iteration inside the loop.
  • Do not use continue within the loop.
  • Do not use try-catch in a loop.
  • Do not search for the same DOM element more than once.
  • Minimize DOM Manipulation.
  • Do not initiate unnecessary layouts.
  • Do not use the in-line method to deal with events.
  • Do not use eval().
  • Do not use with().
  • When using setTimeout or setInterval, do not use literals as callbacks.
  • Do not use new Function() constructor.
  • Do not override or extend native objects.
  • Do not use increment/decrement operators.
  • Do not use this assign.
  • Do not omit the semicolon(;) at the end of each statement.
  • Do not use variables and functions before declaration. (ES5)
  • Do not reference the array.length property for each turn in the iteration. (Legacy)
  • Afterword

✓ Note This document deals with ES5 and higher, and is written based on ES6. Conventions suggested for ES5 are marked with (ES5) tags. For items that only happen in legacy browsers are marked with (Legacy) tags.


Include the <script> tag at the bottom of the document.

Although it is common to group external elements like CSS and JavaScript in the <head> tag of the document, it is not a good practice. When the browser is rendering the page and runs into CSS or JavaScript, it stops rendering, and 1) downloads the file 2) analyzes the syntax 3) compiles. As the file size of the JavaScript file increases, the rendering becomes slower respectively.

▶ Solution
All JavaScript codes should be included inside of the <body> tag and at the end.
<!DOCTYPE html>
<html>
  <head>
    ...
    <!-- Bad: Script is included in the head tag -->
    <script src="../js/jquery-3.3.1.min.js"></script>
    <script src="../js/common.js"></script>
    <script src="../js/applicationMain.js"></script>
  </head>
  <body>
    ...
    <!-- Good: Included at the end of the body tag -->
    <script src="../js/jquery-3.3.1.min.js"></script>
    <script src="../js/common.js"></script>
    <script src="../js/applicationMain.js"></script>
  </body>
<html>

Do not use the direct URL when using external resources.

Most external open source codes like jQuery provide a directly accessible URL. However, when using such URLs, external errors (URL change, CDN malfunction) can directly affect the service, and lead to inconsistent rendering.

▶ Solution
Use corresponding downloaded source files instead of using external URLs.
<!DOCTYPE html>
<html>
  <head>
    <title>HTML Page</title>
  </head>
  <body>
    ...
    <!-- Bad -->
    <script src="https://code.jquery.com/jquery-3.3.1.min.js"></script>

    <!-- Good -->
    <script src="../js/jquery-3.3.1.min.js"></script>
    ...

Do not use global variables.

Like other languages, JavaScript also has a global scope, and in the web browser, the global scope is the window. Undeclared variables and variables declared outside of the function scope become global. Global variable becomes accessible to the entire application. Since it can be overwritten when it is declared using the same name in another module, it requires extra attention.

▶ Solution
Use Namespacing or Immediately Invoked function expression (IIFE).

Namespacing

Namespacing is a practice of creating a global object for the application or the library, and injecting every feature into this object to prevent name collisions. If variables and functions are not declared inside a function or as a property of an object, it is added to the global scope. Define a single global object like MyApp and define necessary variables and functions in the MyApp. This way, the name does not override the window.name and functions like sayName() is called with MyApp.sayName() as to be easily locatable if an error happens.

// Bad - Added name variable and sayName() function to Global
const name = 'Nicholas'; // window.name === 'Nicholas'
function sayName() {
  alert(name);
}

// Good - Defined a global object MyApp and defined name variable and sayName function inside the MyApp
const MyApp = {
  name: 'Nicholas',
  sayName() {
    alert(this.name);
  }
};

MyApp.sayName();

// Tip - Namespace expansion
const nhn = window.nhn || {}; // Used nhn. as the NHN's namespace

// Used the serviceName as a lower secondary namespace
nhn.serviceName = nhn.serviceName || {};

// Used page-groups and feature module-groups as tertiary namespace
nhn.serviceName.util = {...};
nhn.serviceName.component = {...};
nhn.serviceName.model = {...};

// If necessary, expand the namespace into fourth and fifth levels
nhn.serviceName.view.layer = {...};
nhn.serviceName.view.painter = {...};

Immediately Invoked Function Expression

JavaScript functions create a local scope. Variables declared within the function become local variables and are maintained with the function’s life-cycle. Immediately invoked function expressions (IIFEs) are executed only once when invoked. Since it has no reference, it cannot be executed again.

// Bad - today variable is global, so still remains after the code has been reset
const today = new Date();

alert(`Today is ${today.getDate()}`);

// Good - today variable is local to an anonymous function, so cannot be accessed from outside of the IIFE
(function() {
  const today = new Date();

  alert( `Today is ${today.getDate()}`);
})();

alert(today); // Error : today is not defined

// Tip - Variables and functions declared inside of the IIFE can be accessed by returning objects
const nhn = (function() {
  const _privateName = 'NHN';

  function getName() {
    return _privateName;
  }

  function sayHello() {
    return `Hello, ${_privateName}`;
  }

  return {
    whoAmI: getName,
    say: sayHello
  };
})();

nhn.whoAmI(); // NHN
nhn.say(); // Hello, NHN

Always declare import statements at the top.

import statements are used to bring in functions and objects exported by the external modules. Modules that have been brought in using import statements are called dependency modules. ES6 modules allow all dependency modules to load before the codes execute. Although dependency modules do not cause any errors even if they are used before declared, the point of import can cause readability issues and hinders developers from knowing which modules have been imported.

▶ Solution
Always declare import statements at the top.
// Bad
foo.init();
bar.getName();

import foo from 'foo';
import bar from 'bar';


// Good - Declared at the top
import foo from 'foo';
import bar from 'bar';
...
foo.init();
bar.getName();

Do not use variables without declaration.

Variables declared without const, let, or var keywords are considered to be global. Although it does not cause any errors (strict mode does,) it pollutes the global environment and sometimes cause bugs that are incredibly difficult to find. var keyword takes the function scope and is hoisted before the code is executed, so does not raise any errors even if used before declared. However, variables declared using const and let are block scoped and are not hoisted. Since these are affected by Temporal Dead Zone (TDZ), these variables will raise errors if called within the same block and before declaration.

▶ Solution
When declaring variables, always use 'const', 'let', and 'var'.
In environments that do not support 'const' and 'let', use 'var' to declare variables.
// Bad
const foo = 'foo';
let bar = 'bar';
baz = 'var'; // baz is declared globally, and pollutes the global environment.

// Good
const foo = 'foo';
let bar = 'bar';
let baz = 'var';

Do not use constructors when creating arrays and objects.

While it is possible to use constructor functions to declare arrays and objects, literal declaration is much more concise, intuitive, and faster. Actually, JavaScript engine is optimized to run literal statements. If a number is passed as a parameter to the array constructor, it could mistake it for the length of the array, so it is important to pay extra attention to ensure that it functions as intended.

▶ Solution
Use literal statements to declare objects and arrays.
// Bad
const emptyArr = new Array();
const emptyObj = new Object();

const arr = new Array(1, 2, 3, 4, 5);
const obj = new Object();
obj.prop1 = 'val1';
obj.prop2 = 'val2';

const arr2 = new Array(5);
arr2.length // 5

// Good
const emptyArr = [];
const emptyObj = {};

const arr = [1, 2, 3, 4, 5];
const obj = {
  prop1: 'val1',
  prop2: 'val2'
};

Note

Do not use the == to check for equality.

Before checking or executing arithmetic operations, JavaScript first executes an implicit type coercion. This enables JavaScript to operate with different types of variables, but implicit type coercion may complicate the code as a whole and sometimes buries type errors caused in arithmetic operations. In order to use the == operator properly, the author and the reader of the program must fully understand the underlying concepts of type coercion.

▶ Solution
Use === or !== so that implicit type coercion does not happen.
If a comparison between different types of variables must be made, convert the types intentionally and then use === or !== to compare.
// Bad
undefined == null // true
123 == '123' // true
true == 1 // true
false == 0 // true

// Good
123 === '123' // false
Number('123') === 123 // true
String(123) === '123' // true
1 === true // false
0 === false // false
Boolean(1) === true // true
Boolean(0) === false // true
undefined === null // false
Boolean(undefined) === Boolean(null) // true

// Explicit type change
Number('10') === 10
parseInt('10', 10) === 10
String(10) === '10'
+'10' === 10
(123).toString() === '123'
Boolean(null) === false
Boolean(undefined) === false

Note

Do not omit the curly braces ({…}).

Even if the if/while/do/for statements are single-line statements, it is better not to omit the curly braces ({…}). Without the curly braces, it is difficult to tell the functional scope of the code.

▶ Solution
Do not omit the curly braces even for single line statements.
// Bad
if(condition) doSomething();

if (condition) doSomething();
else doAnything();

for(let prop in object) someIterativeFn();

while(condition) iterating += 1;

// Good
if (condition) {
  doSomething();
}

if (condition) {
  doSomething();
} else {
  doAnything();
}

for (let prop in object) {
  someIterativeFn(object[prop]);
}

while (condition) {
  iterating += 1;
}

Note

Do not omit the second parameter, base, of parseInt.

parseInt is a function that changes the string into a numeral equivalence. The first parameter is the target string to be changed and the second parameter is the base. If the base if omitted, the browser will independently decide on which base to use, and each browser can decide on different bases. (If the string starts with ‘0x’ or ‘0X’, hexadecimal is used, and if it starts with ‘0’, uses either octal or decimal.)

▶ Solution
Prevent any possible errors by specifying the base.
If decimal conversion is needed, it is better in terms of speed to use the 'Number()' function.
// Bad
const month = parseInt('08');
const day = parseInt('09');

// Good
const month = parseInt('08', 10);
const day = parseInt('09', 10);

// Tip : If converting to decimals, using Number() function or '+' operator is faster.
const month = Number('08')
const day = +'09';

Note

Do not omit the break clauses in switch statements.

During a switch statement, each case will exit the case clause when it runs into the break keyword. Without the break keyword, it will continue on to the next case clause, and this could lead to confusion among readers because reader has no way of knowing if this was intentional or was simply a mistake. For example, in the first case clause, A() function is called, and without a break, the code will proceed to execute the function B() in the second case clause. In this case, the code will execute both functions A() and B(). Such composition of codes affects the readability negatively, and even if the break keyword has been left out as a mistake, an error that is extremely difficult to find, rises. Therefore, a break keyword should always be included in case clauses. However, this does not apply to a case where the code is designed to have multiple case clauses execute.

▶ Solution
Do not omit 'break'. (The 'break' keyword can be omitted if multiple cases perform the same task.)
Use a 'default' clause to represent a 'case' where it does not apply to any of the cases.
// Bad - break is omitted, so at case 2, both functions A() and B() will have been executed.
switch (foo) {
  case 1:
    A()
  case 2:
    B();
    break;
  ...
}

// Good
switch (foo) {
  case 1:
    A()
    break;
  case 2:
    C();  // C() function performs both tasks included in A() and B()
    break;
  ...
}

// Good - If multiple cases perform the same task, break can be omitted
switch (foo) {
  case 1:
  case 2:
    doSomething();
    break;
  ...
}

// Good
switch (foo) {
  case 1:
    doSomething();
    break;
  case 2:
    doSomethingElse();
    break;
  ...
  default:
    defaultSomething();
}

Note

Do not use for-in to iterate through an array.

In order to iterate through an object’s properties, for-in is used. Although array is also an object, to iterate through an array, for-in must not be used. Since for-in is used to iterate through every property in the prototype chain, it is much slower than using for. Also, different browsers define the iteration sequence differently, so it is possible for the array iteration to happen in the order not specified by the index.

▶ Solution
Use for statement to iterate through an array.
// Bad
const scores = [70, 75, 80, 61, 89, 56, 77, 83, 93, 66];
let total = 0;

for (let score in scores) {
  total += scores[score];
}

// Good
const scores = [70, 75, 80, 61, 89, 56, 77, 83, 93, 66];
let total = 0;
const {length} = scores;

for (let i = 0; i < length; i += 1) {
  total += scores[i];
}

Do not use delete to delete an element from an array.

Usually to delete an object’s property, delete is used. In this case, the element is not set to undefined, but is completely deleted so that the element does not exist. In JavaScript, array is also a type of an object, so the delete method can be used, but it functions a little bit differently with arrays. One might think that the delete method makes the array’s element completely disappear, as is the case with regular objects, but it does not. The element is set to undefined, so the length of the array after having an element deleted stays the same.

▶ Solution
Use Array.prototype.splice() to remove an element from the array or use 'length' to restrict the length of an array.
// Bad
const numbers = ['zero', 'one', 'two', 'three', 'four', 'five'];
delete numbers[2]; // ['zero', 'one', undefined, 'three', 'four', 'five'];

// Good
const numbers = ['zero', 'one', 'two', 'three', 'four', 'five'];
numbers.splice(2, 1); // ['zero', 'one', 'three', 'four', 'five'];

// Tip - To restrict the length of an array, use length.
const numbers = ['zero', 'one', 'two', 'three', 'four', 'five'];
numbers.length = 4; // ['zero', 'one', 'two', 'three'];

Do not process any task unrelated to the iteration inside the loop.

The loop will continue as long as the given expression holds true. Since loops affect the performance of the code greatly, the optimization for loops is often the first thing to be considered when refactoring. It warrants extra attention to make sure that the loop does not include any unnecessary operations like returning the same value over and over again.

▶ Solution
Optimize the loop so that only necessary operations take place.
// Bad
for (let i = 0; i < days.length; i += 1) {
  const today = new Date().getDate();
  const element = getElement(i);

  if (today === days[i]) {
    element.className = 'today';
  }
}

// Good
const today = new Date().getDate();
const {length} = days;
let element;

for (let i = 0; i < length; i += 1) {
  if (today === days[i]) {
    element = getElement(i);
    element.className = 'today';
    break;
  }
}

Do not use continue within the loop.

To use Douglas Crockford’s words, there is not ”a piece of code that was not improved by refactoring it to remove the continue statement”. If continue statement is used in a loop, JavaScript engine creates a separate context in which to manage the statement. Such loops affect the performance of the entire application, so should be avoided. If used correctly, continue statements make codes more succinct, but if overused, make codes confusing and hard to maintain.

▶ Solution
To skip some procedures inside of a loop, use conditionals.
// Bad
let loopCount = 0;

for (let i = 1; i < 10; i += 1) {
  if (i > 5) {
    continue;
  }
  loopCount += 1;
}

// Good
for (let i = 1; i < 10; i += 1) {
  if (i <= 5) {
    loopCount += 1;
  }
}

Do not use try-catch in a loop.

Try-catch is often used to deal with exceptions. If the try-catch is used within a loop, every time the loop is executed, a new variable designed to store exception object is created in the current scope.

▶ Solution
Create a function wrapping the 'try-catch', and call the function inside of the loop.
// Bad
const {length} = array;

for (let i = 0; i < length; i += 1) {
  try {
    ...
  } catch (error) {
    ...
  }
}

// Good
const {length} = array;

function doSomething() {
  try {
    ...
  } catch (error) {
    ...
  }
}

for (let i = 0; i < length; i += 1) {
  doSomething();
}

Do not search for the same DOM element more than once.

getElementById, getElementByTagName, and querySelector are all APIs used to search for a DOM element. Since it can be costly to search for the DOM element, repeatedly searching for the same element could take a toll on the performance.

▶ Solution
Economize the resources by caching the DOM elements that have been searched.
// Bad
const className = document.getElementById('result').className;
const clientHeight = document.getElementById('result').clientHeight;
const scrollTop = document.getElementById('result').scrollTop;
document.getElementById('result').blur();

// Good
const el = document.getElementById('result');
const {className, clientHeight, scrollTop} = el;
el.blur();

Minimize DOM Manipulation.

When either innerHTML or appendChild method is called, the DOM has to be changed. DOM tree reconstruction can be exorbitant, it is recommended to only construct the DOM tree once, instead of repeatedly manipulating it.

▶ Solution
If the code requires the DOM to be changed, apply all of the change at once, as to minimize the number of DOM tree reconstruction.
const el = document.getElementById('bookmark-list');

// Bad
myBookmarks.forEach(bookmark => {
  el.innerHTML += `<li><a href="${bookmark.url}">${bookmark.name}</a></li>`;
});

// Good
const html = myBookmarks
  .map(bookmark => `<li><a href="${bookmark.url}">${bookmark.name}</a></li>`)
  .join('');

el.innerHTML = html;

Do not initiate unnecessary layouts.

Layout (also known as reflow in Firefox) is a process where the render tree is reconstructed due to a geometric change (width, height, location) of any node (self, child, parent, root) inside of the DOM node created by the browser engine.

Layout occurs when

  • the page is rendering for the first time.
  • the window resizing happens.
  • node is added or deleted.
  • element size or location has been changed.
  • certain properties (offsetHeight, offsetTop…) is read and used.
  • certain methods (getClientRects(),getBoundingClientRects()…) are called.

Forceful layouts happening multiple times consecutively is called layout thrashing, and since browsers have to recalculate redundantly, it negatively influences the overall performance of the application.

▶ Solution
If layout thrashing is absolutely necessary inside a loop, use the cache value from outside of the loop.
// Bad
function resizeWidth(paragraphs, box) {
  const {length} = paragraphs;

  for (let i = 0; i < length; i += 1) {
    paragraphs[i].style.width = `${box.offsetWidth}px`;
  }
}

// Good
function resizeWidth(paragraphs, box) {
  const {length} = paragraphs;
  const width = box.offsetWidth;

  for (let i = 0; i < length; i += 1) {
    paragraphs[i].style.width = `${width}px`;
  }
}

Note

Do not use the in-line method to deal with events.

In the early stages of JavaScript development, listeners were registered to DOM elements using the in-line method. In the example code below, doSomething() is a function from an external JavaScript file. If the doSomething function had to be changed in any way, such as changing the name of the function or changing the callback function, the entire HTML file had to be changed accordingly. Such creates dependent relationship between JavaScript and HTML so small changes lead to wide range of corrections, and thereby increases the complexity of debugging and the cost of maintenance. Also, in-line method prevents developers to add more than one event listener to an element.

▶ Solution
Separate the in-line JavaScript codes from HTML
<!-- Bad -->
<button onclick="doSomething()" class="action-btn">Click Me</button>

<!-- Good -->
<button class="action-btn">Click Me</button>

...
<script src="js/btn-event.js"></script>
// btn-event.js
const btn;

function doSomething() {
  ...
}
...
// Good
btn = el.querySelector('.action-btn');
btn.addEventListener('click', doSomething, false);

Do not use eval().

eval() is a function that executes the JavaScript code that is written in string format. The eval() function takes the string as its parameter and immediately executes the string in the caller’s local scope. Variables and functions declared within eval function do not materialize in accordance to the structure analysis, but at the point the eval function is used. Therefore, a structure analyzer must be called in the middle of the execution process, and this creates enormous stress on the program and significantly slows down the program. Also, by using eval() to verify strings passed in from a user or a network could lead to major security flaws.

▶ Solution
Never use 'eval()'.
Same can be achieve by coding differently.
const nhn = {
  name: 'NHN',
  lab: 'FE Dev Lab',
  memberCount: 10
};
const propName = 'name';

// Bad
eval(`nhn.${propName}`); // NHN

// Good
nhn[propName] // NHN

Note

Do not use with().

Although with() offers a little bit of convenience when accessing certain objects repeatedly, it creates unintended errors, and it is recommended not to use it. The following is an example of with usage.

function doSomething(value, obj) {
  ...
  with(obj) {
    value = 'which scope is this?'
  }
}

The same can be done using the following code.

value = 'which scope is this?';
obj.value = 'which scope is this?';

It is difficult to tell how the code was executed just by examining the code. It can be executed differently every time it is ran, and it is also possible to have been changed as it is running. It is almost impossible to understand the intent of the code and even more so to predict the outcome of such codes. Therefore, using with() cannot guarantee that the code will run as the developer intended it. Furthermore, the with statement establishes a new scope every time it runs, costing more resources, and interrupts internal Search Engine Optimization of JavaScript, making the program to run much slower.

▶ Solution
If a certain object has to be accessed repeatedly, use a cache variable.
// Bad
with(document.getElementById('myDiv').style) {
  background = 'yellow';
  color = 'red';
  border = '1px solid black';
}

// Good
const {style} = document.getElementById('myDiv');
style.background = 'yellow';
style.color = 'red';
style.border = '1px solid black';

Note

When using setTimeout or setInterval, do not use strings as callbacks.

setTimeout and setInterval functions both execute the callback function taken in as the first parameter after a certain period of time. Although the first parameter can be passed in as a string, it hinders the performance because it is executed as eval internally.

▶ Solution
When using 'setTimeout', 'setInterval', use the callback functions directly.
// Bad
function callback() {
  ...
}

setTimeout('callback()', 1000);

// Good (1)
function callback() {
  ...
}

setTimeout(callback, 1000);

// Good (2)
setTimeout(() => {
    ...
}, 1000);

Note

Do not use new Function() constructor.

While it is not often used, the function constructors can be used to define functions. In this case, strings passed in as parameters have to be translated using the eval function, and it slows down the entire performance.

▶ Solution
When declaring functions, use function declaration or function expression.
// Bad
const doSomething = new Function('param1', 'param2', 'return param1 + param2;');

// Good (1) - function declaration
function doSomething(param1, param2) {
  return param1 + param2;
}

// Good (2) - function expression
const doSomething = function(param1, param2) {
  return param1 + param2;
};

Note

Do not override or extend native objects.

JavaScript is dynamic by nature, properties of previously defined objects can be added, deleted, and changed. Using Object.defineProperty will allow users to declare property attributes, and with the right configuration, it is also possible to prevent others to add, delete, and change the properties. In JavaScript, it is allowed to add new properties to the native object’s prototypes and even redefine original properties. However, by overriding or extending the native objects, it can lead to confusion in other developers expecting the native object to behave normally. It could also lead to a data collision, where a method is supported in one browser, but is not in another. It is quite possible to override the native method by mistake, and such mistake could lead to unpredictable errors.

▶ Solution
Never change the native objects.
If native functions are needed, write a new function that performs the same task, or create a new function to work with the native feature.
const o = {
  x: 1,
  y: 2
};

// Bad
Object.prototype.getKeys = function() {
  const keys = [];

  for (let key in this) {
    if (this.hasOwnProperty(key)) {
      keys.push(key);
    }
  }

  return keys;
};

o.getKeys();

// Good
function getKeys(obj) {
  const keys = [];

  for (let key in obj) {
    if (obj.hasOwnProperty(key)) {
      keys.push(key);
    }
  }

  return keys;
}

getKeys(o);

Note Monkey Patching Monkey patching is extending a native function or an object. However, monkey patching disturbs the capsulation, and pollutes the native object by adding features that are not standard, so it is recommended not to use it. Despite these dangers, there is one situation where monkey patching can be used, and it is called polyfill. Polyfill is an act of substituting a function that does not yet exist in the current version of JavaScript like Array.protytype.map with a function that is similar to it. Uses of monkey patching other than polyfill, to enhance compatibility of JavaScript, should be prohibited.

// Example of polyfill
if (typeof Array.prototype.map !== 'function') {
  Array.prototype.map = function(f, thisArg) {
    const result = [];
    const {length} = this;

    for (let i = 0; i < length; i += 1) {
      result[i] = f.call(thisArg, this[i], j);
    }

    return result;
  };
}

Do not use increment/decrement operators.

Increment/decrement operators make it hard to tell whether the operation or assignment comes first.

▶ Solution
Use the value assignment to write readable codes.
let num = 0;
const {length} = arr;

// Bad
for (let i = 0; i < length; i++) {
     num++;
}

// Good
for (let i = 0; i < length; i += 1) {
    num += 1;
}

Note

Do not use this assign.

The value of this is determined when the code executes. If a function calls another function inside it, that function’s this and the higher function’s this are not the same. When writing the source code, there may be occasions when a reference has to be made to the this of higher context. Similar reference variables (that, self, me…) can be used as closures to reference the higher context, but it can cause confusion among developers.

▶ Solution
Instead of declaring new reference variables, use Function.prototype.bind function or arrow functions.
// Bad
function() {
  const self = this;

  return function() {
    console.log(self);
  };
}

function() {
  const that = this;

  return function() {
    console.log(that);
  };
}

function() {
  const _this = this;

  return function() {
    console.log(_this);
  };
}

// Good
function printContext() {
  return function() {
    console.log(this);
  }.bind(this);
}

function printContext() {
  return () => console.log(this);
}

Do not omit the semicolon(;) at the end of each statement.

Do not omit the semicolon(;) at the end of each statement. Although JavaScript automatically inserts a semicolon at the end, it could lead to mean something completely unintended. Also, it puts pressure on the JavaScript engine to locate the missing semicolon and to place it, so it slows down the process.

▶ Solution
Use a semicolon at the end of each statement.

Do not use variables and functions before declaration. (ES5)

In versions prior to ES5, block scope does not exist even if block structures are used. If a variable is declared using the var variable, it is hoisted up to the top of the function scope no matter where it was declared, so it can be used anywhere within the function scope. Such hoisting works on both variables declared with var and functions declared with function keywords. These hoisted variables can cause code readability issues, and can cause errors that are hard to find.

▶ Solution
Declare functions before it is used, and declare variables with 'var' keywords at the top.
// Bad
doSomething();

function doSomething() {
  foo1 = foo2;
  ...
  var foo1 = 'value1';
  foo3 = foo4;
  ...
  var foo3;
  ...
  var foo4 = 'value4';
  var foo2;
}

// Good
function doSomething() {
  var foo1 = 'value1';
  var foo4 = 'value4';
  var foo3 = foo4;
  var foo2;
  ...
  foo1 = foo2;
}

doSomething();

Do not reference the array.length property for each turn in the iteration. (Legacy)

The for loop will continue to execute as long as the given expression turns out to be true. This means if there have been 100 iterations, the same expression was executed 100 times. Therefore, in for loops, using array.length attribute to operate the loop is extremely ineffective. (While in modern browsers, not saving the array.length value to the cache does not make much difference, in older browsers like IE10, performance difference can be devastating.)

▶ Solution
Cache the array.length values to operate for loops in older browsers.
// Bad
for (var i = 0; i < array.length; i += 1) {
  ...
}

// Good
var len = array.length;

for (var i = 0; i < len; i += 1) {
  ...
}

Afterword

This document addressed anti-patterns. Anti-pattern is a set of patterns many developers habitually use, but rejected for the sake of performance, debugging, maintenance, and readability. This document explains each anti-pattern with examples and provide suggestions for improvement. The author hopes that this document helps readers build correct coding habits and reject the use of anti-patterns.


This document is an official Web Front-End development guide written and maintained by NHN FE Development Lab. Any errors, questions, and points of improvement pertaining to this document should be addressed to the official support channel (dl_javascript@nhn.com).


Last Modified
2019. 04. 10

FE Development LabBack to list