Categories
JavaScript Mistakes

JavaScript Mistakes — Duplicates and Useless Code

JavaScript is a very forgiving language. It’s easy to write code that runs but has mistakes in it.

In this article, we look at some mistakes that people make with conditionals and code blocks.

Duplicate Conditions in if-else-if Chains

We should be careful not to have 2 or more else if blocks with the same condition. This leads to confusion and it’s almost always a mistake. They’ll all evaluate to the same truth value and the later ones will never run.

For instance, if we have:

let x;
const a = false;
const b = true;
if (a) {
  x = 0;
} else if (b) {
  x = 1;
} else if (b) {
  x = 2;
}

Then we’ll see that x is 1 because of the short-circuit evaluation. b is true , so that first else if block that checks for b is run.

Therefore, we should always have different conditions for each else if block. For instance, we should write:

let x;
const a = false;
const b = false;
const c = true;
if (a) {
  x = 0;
} else if (b) {
  x = 1;
} else if (c) {
  x = 2;
}

We should also be careful of duplicates that are caused because of the || and && operator. For instance:

let x;
const a = false;
const b = true;
if (a) {
  x = 0;
} else if (b) {
  x = 1;
} else if (a || b) {
  x = 2;
}

It’s also a duplicate because the last if block’s condition is either the same as a or b .

Duplicate Keys in Object Literals

Duplicate keys should never be in object literals. It’s more typing that doesn’t do any good.

For example, the following object:

const foo = {
  bar: 1,
  bar: 2
};

is the same as { bar: 2 } . The first one is discarded. Other examples include:

const foo = {
  bar: 1,
  'bar': 2
};

or:

const foo = {
  1: 'foo',
  '1': 'bar'
};

Duplicate Case Labels

We shouldn’t have duplicate case labels in our switch statements. For instance, if we have:

const a = 1;
let foo = '';

switch (a) {
  case 1:
    foo = 'a';
    break;
  case 2:
    foo = 'b';
    break;
  case 1:
    foo = 'c';
    break;
  default:
    break;
}

Then foo is 'a' since the switch statement stops evaluating when it encounters the first matching case.

Therefore, the duplicate case label is useless. Instead, we should write:

const a = 1;
let foo = '';

switch (a) {
  case 1:
    foo = 'a';
    break;
  case 2:
    foo = 'b';
    break;
  default:
    break;
}

Empty Block Statements

Empty block statements are useless. Therefore, we shouldn’t include them. For instance:

if (x === 1) {}

We should run something if we have a block.

Empty Character Class in Regex

An empty character class in regex doesn’t match anything, so it probably shouldn’t be in the code. For instance, we write:

const foo = /^foo[]/;

The [] doesn’t do anything so it should be removed.

Reassigning Exceptions in catch Clauses

We shouldn’t reassign the exception object passed in from catch to something else since it obscures the source of the original error. If we assign it to something else, then we wouldn’t be able to determine the origin of the error from this point on.

For instance, if we write:

try {
  // ...
} catch (e) {
  e = 'foo';
}

Then e becomes 'foo' after the assignment. Then we won’t know what e is originally after the reassignment.

Instead, we should assign 'foo' to another variable as follows:

try {
  // ...
} catch (e) {
  let foo = 'foo';
}

Then e has the same value and before and we can also reference 'foo' .

Conclusion

We should never have duplicate conditions in multiple else-if blocks since only the first one in the group of duplicates will do something. This also applies to case labels.

Object key literals also shouldn’t be duplicated since only the last one’s value will remain.

Other useless codes include empty code blocks and empty regex classes.

Finally, we shouldn’t reassign the exception parameter to another value since the original exception value will begone. Instead, we should create a new variable and assign whatever we want to it.

Categories
JavaScript Mistakes

JavaScript Mistakes — Useless Promise Code and Operations

JavaScript is a very forgiving language. It’s easy to write code that runs but has mistakes in it.

In this article, we’ll look at some redundant code that we should remove from our code that we may put in our async functions and other places.

No Unnecessary return await

In JavaScript, async functions always return promises. If we return something there, we return a promise with the resolved value being what we return in the promise.

return await doesn’t do anything except adding extra time before the promise resolves or rejects.

Therefore, we can just return the promise directly.

The only valid use of return await is in a try...catch statement to catch errors from another function that returns a promise.

For instance, the following code is bad:

const foo = async () => {
  return await Promise.resolve(1);
}

as we don’t need the await to return the promise.

We can instead just write:

const foo = async () => {
  return Promise.resolve(1);
}

However, the following use of return await is good:

const foo = async () => {
  try {
    return await Promise.resolve(1);
  } catch (ex) {
    console.log(ex);
  }
}

No Script URLs

Having javascript: URL is a bad practice because it exposes us to the same performance and security issues as eval .

They both let us run arbitrary code from a string, so attackers can do the same.

Also, the performance from running code from a string is slower since the JavaScript interpreter can’t optimize code that’s in a string.

For instance, we should write code like:

location.href = "javascript:alert('foo')";

Instead, just put it inside the JavaScript code as follows:

alert('foo');

No Self Assignment

Self assignments are useless. Therefore, it shouldn’t be in our code.

It’s probably an error that we haven’t caught when we changed our code.

For instance, the following are useless:

a = a;
[a, b] = [a, b];

Instead, we should write something like:

a = b;
[a, b] = [b, a];

No Self Compare

Comparing a variable to itself always returns true . Therefore, it’s pretty useless to have these kinds of expressions in our code.

It’s usually an error from typing or refactoring our code. This may introduce runtime errors in our code.

The only time that it may be appropriate to compare a value against itself is when comparing NaN since NaN doesn’t equal itself when compared with the === operator.

But it’s better to check for NaN using the isNaN or Number.isNaN functions to check for NaN .

isNaN tries to convert its argument to a number before checking for NaN , while Number.isNaN just check for the value without doing anything data type conversion first.

Therefore, we shouldn’t have code like the following:

const a = 1;
`if (a === a) {
    x = 2;
}`

Don’t Use the Comma Operator

The comma operator always returns the last element from the list of items separated by the operator, evaluating them from left to right.

Therefore, it’s not a very useful operator to be using in our code.

We should probably remove any use of it from our code. So something like:

const a = (1, 2, 3);

will always set a to 3.

There aren’t any valid use cases for this operator in a normal program.

Photo by Daiji Umemoto on Unsplash

Only Throw Error Object in throw Expressions

In JavaScript, we can put any value after throw when we try to throw errors.

However, it isn’t good practice to throw anything other than the Error object or a child constructor derived from it.

The Error object automatically keep track of where the Error object was built and originated.

For instance, we should have throw expressions like:

throw "fail";

Instead, we should write:

throw new Error("fail");

Conclusion

return await is mostly useless except for try...catch blocks. Comparing a variable or value to itself is mostly useless, as with assign a variable to itself.

Likewise, the comma operator is also useless as it always returns the last value in the list.

Errors that throw exceptions should always throw an Error object or an instance of its child constructor.

Script URLs in JavaScript is also a bad practice because we’re running code within a string, which prevents any optimizations from being done and it may let attackers run arbitrary with our code.

Categories
JavaScript Mistakes

JavaScript Mistake — Loops, Promises, and More

JavaScript is a very forgiving language. It’s easy to write code that runs but has mistakes in it.

In this article, we’ll look at some JavaScript mistakes, including loops and promises.

Wrong For Loop Direction

A for loop with an ending condition that’ll never be reached is probably buggy code. If we want to make an infinite loop, we should use a while loop as the convention.

For instance, if we have:

for (let i = 0; i < 20; i--) {
}

It’s probably a mistake because we specified the ending condition, but never reach it.

We probably meant:

for (let i = 0; i < 20; i++) {
}

Getters That Don’t Return Anything

If we make a getter but it doesn’t return anything, it’s most likely a mistake. There’s no reason to make a getter that returns undefined .

For instance, the following is probably incorrect:

let person = {
  get name() {}
};

Object.defineProperty(person, "gender", {
  get() {}
});

class Person {
  get name() {}
}

We have useless getters in all of the code above. If we have a getter, then we should return something in it:

let person = {
  get name() {
    return 'Jane';
  }
};

Object.defineProperty(person, "gender", {
  get() {
    return 'female';
  }
});

class Person {
  get name() {
    return 'James';
  }
}

Async Function as a Promise Executor Callback

When we define a promise from scratch, we have to pass in an executor callback function with the resolve and reject functions as parameters.

We don’t want async functions as executors because when errors are thrown, they’ll be lost and won’t cause the newly constructed promise to reject. This makes debugging and handling some errors hard.

If a promise executor function is using await , then it’s usually a sign that creates a new Promise instance is useless or the scope of the new Promise constructor can be used.

What is using await is already a promise and async functions also return a promise, so we don’t need a promise inside a promise.

For example, if we have the following code:

const fs = require('fs');

const foo = new Promise(async (resolve, reject) => {
  fs.readFile('foo.txt', (err, result)=> {
    if (err) {
      reject(err);
    } else {
      resolve(result);
    }
  });
});

const result = new Promise(async (resolve, reject) => {
  resolve(await foo);
});

Then it’s probably a mistake because we’re nesting a promise inside a promise.

What we actually want to do is:

const fs = require('fs');

const foo = new Promise(async (resolve, reject) => {
  fs.readFile('foo.txt', (err, result)=> {
    if (err) {
      reject(err);
    } else {
      resolve(result);
    }
  });
});

const result = Promise.resolve(foo);

Photo by Matthew Henry on Unsplash

No Await Inside Loops

async and await allows for parallelization. Usually, we want to run Promise.all to run unrelated promises in parallel. Running await in a loop will run each promise in sequence. This isn’t necessary for promises that don’t depend on each other.

For instance, we should write:

const bar = (results) => console.log(results);

const foo = async (arr) => {
  const promises = [];
  for (const a of arr) {
    promises.push(Promise.resolve(a));
  }
  const results = await Promise.all(promises);
  bar(results);
}

Instead of:

const bar = (results) => console.log(results);

const foo = async (arr) => {
  const results = [];
  for (const a of arr) {
    results.push(await  Promise.resolve(a));
  }
  bar(results);
}

The first example is a lot faster than the second since we’re running them in parallel instead of sequentially.

If the promises are dependent on each other, then we should use something like the 2nd example.

Don’t Compare Anything Against Negative Zero

Comparing again negative zero will return true for both +0 and -0. We probably actually want to use Object.is(x, -0) to check if something is equal to -0.

For instance, in the following code:

const x = +0;
const y = -0;
console.log(x === -0)
console.log(y === -0)

Both expressions will log true . On the other hand, if we use Object.is as follows:

const x = +0;
const y = -0;
console.log(Object.is(x, -0))
console.log(Object.is(y, -0))

Then the first log is false and the second is true , which is probably what we want.

Conclusion

There’re many ways to write code that works unintentionally with JavaScript. To prevent bugs from occurring, we should use Object.is to compare again +0 and -0, running promises inside promise executor callbacks, adding getters that don’t return anything, or creating infinite loops unintentionally.

If promises can be run in parallel, then we should take advantage of that by using Promise.all .

Categories
JavaScript Mistakes

JavaScript Mistakes — Expressions

JavaScript is a very forgiving language. It’s easy to write code that runs but has mistakes in it.

In this article, we’ll look at some confusing expressions that we shouldn’t be writing in JavaScript code.

Confusing Multiline Expressions

JavaScript has the automatic semicolon insertion(ASI) feature which adds semicolons automatically.

Therefore, we can omit the semicolon and still have a valid JavaScript code. However, this doesn’t mean that they’re easy to read for users.

In JavaScript, a newline character always ends a statement like with a semicolon except when:

  • The statement has an unclose parentheses, array literal, object literal or ends in some other way that’s not a valid way to end a statement
  • The lines is -- or ++
  • It’s a for , while , do , if , or else and there’s no (
  • The next lines start with arithmetic or other binary operators that can only be found between 2 operands

There’re cases where multiline expressions where the new line looks like it’s ending a statement, but it’s not. For instance, the following aren’t are multiline but are actually one expression:

let b = 3
let a = b
(1 || 2).c();

We’ll get a ‘b is not a function’ message since the last 2 lines are interpreted as:

let a = b(1 || 2).c();

Another would be the following:

let addNumber = ()=>{}
let foo = 'bar'
[1, 2, 3].forEach(addNumber);

The code above would get us syntax errors. Therefore, we should put semicolons at the end of each statement so that no developer or JavaScript interpreter would be confused or give errors.

Unreachable Code After return, throw, continue, and break statement

Unreachable code after return , throw , break , and continue are useless because these statements unconditionally exits a block of code.

Therefore, we should never have code that’s never going to be run after those lines.

For instance, the following function has unreachable code:

const fn = () => {
  let x = 1;
  return x;
  x = 2;
}

x = 2 is unreachable since comes after the return statement.

Other pieces of code that we shouldn’t write include:

while(true) {
    break;
    console.log("done");
}

const fn = () => {
  throw new Error("error");
  console.log("done");
}

Therefore, we should write:

const fn = () => {
  let x = 1;
  return x;
}

Control Flow Statements in finally Blocks

In JavaScript, the finally block is always run before the try...catch block finishes when it’s added after a try...catch block. Therefore, whenever we have return , throw , break , continue in finally , the ones in try...catch are overwritten.

For instance, if we have:

let x = (() => {
  try {
    return 'foo';
  } catch (err) {
    return 'bar';
  } finally {
    return 'baz';
  }
})();

Then x would be 'baz' since the finally block’s return statement runs before the ones in the try...catch block.

Therefore we shouldn’t add flow control statements to the finally block since it made the ones in try...catch useless. We should instead write something like:

let x = (() => {
  try {
    return 'foo';
  } catch (err) {
    return 'bar';
  } finally {
    console.log('baz');
  }
})();

so that the return statements in try...catch have a chance to run.

Negating the Left Operand of Relational Operators

Negating the left operand doesn’t always negate the whole operation expression in JavaScript.

For instance:

!prop in object

only negates prop and:

!foo instanceof C

only negates foo .

Therefore, !prop in object is actually true in object or false in object depending on the truthiness of prop .

Likewise, !foo instanceof C is actually, true instanceof C or false instanceof C depending on the truthiness of foo.

Therefore, we should wrap the whole expression in parentheses so it actually negates the return value of the whole expression as follows. Therefore:

!(`prop in object)`

and:

!(`foo instanceof C)`

are what we want.

This way, there’s no confusion about what we’re trying to do with those expressions.

Conclusion

There’re many ways to create confusing expressions with JavaScript. One way is to omit semicolons at the end of the line. Omitting semicolons can create ambiguous expressions easily.

We can also create useless code by writing unreachable expressions. They’re useless so they shouldn’t be written. This include writing code after return , break , throw , and continue . Also, writing those statements in finally also make the ones in try...catch useless.

Finally, negating the left operand of the in and instanceof operators only negate the left operand rather than the whole expression.

Categories
JavaScript Mistakes

JavaScript Mistakes — Spaces and Useless Code

JavaScript is a very forgiving language. It’s easy to write code that runs but has mistakes in it.

In this article, we look at how to eliminate useless characters that may break code.

Irregular Whitespace Characters

There’re whitespace characters that aren’t considered whitespaces in all places. Therefore, we may get Unexpected token errors.

Also, it’s not shown in modern browsers, which makes code hard to visualize properly.

Line separators are also invalid characters within JSON which cause more parse errors.

The full list of irregular characters is here.

Characters with Multiple Code Points in Character Class Syntax

Characters with multiple code points, like U+2747 aren’t allowed in strings. We should use characters which are composed of single code points like regular alphanumeric characters and punctuation symbols in our strings.

Calling Global Object Properties as Functions

Some global object properties like Math , JSON , and Reflect shouldn’t be called functions since only their methods are meant to be called and properties are meant to be accessed.

For instance, the following are invalid:

let math = Math();
let json = JSON();

The correct way to use the objects above are to access their properties:

let pi = `Math.PI;
let obj = JSON.parse('{}');`

Using Object.prototypes Builtins Directly

Some methods from the Object.prototype like hasOwnProperty , isPrototypeOf , and propertyIsEnumerable aren’t meant to be called directly by the instance.

For instance, the following are incorrect:

let bar = {};
let foo = {};
const hasBarProperty = foo.hasOwnProperty("bar");
const isPrototypeOfBar = foo.isPrototypeOf(bar);
const barIsEnumerable = foo.propertyIsEnumerable("bar");

They’re meant to be called as follows:

let bar = {};
let foo = {};
const hasBarProperty = Object.prototype.hasOwnProperty.call(foo, "bar");
const isPrototypeOfBar = Object.prototype.isPrototypeOf.call(foo, bar);
const barIsEnumerable = Object.prototype.propertyIsEnumerable(foo, "bar");

Extra Whitespaces in Regular Expressions Literals

Multiple whitespaces are hard to read. It’s hard to tell how many whitespaces are inside the regex. Therefore, we should use single whitespace and then specify the number of whitespaces we’re intending to match.

For instance, instead of writing:

const re = /a   b/;

We should write:

const re = /a {3}b/;

Returning Values from Setters

Setters’ return value is ignored even if it’s there. Therefore, the return value is useless inside the setter. It also creates more chances for error.

This applies to object literals, class declarations and expressions, and properties descriptors in Object.create , Objecr.defineProperty , Object.defineProperties and Reflect.defineProperty.

For instance, we should return values in the following code:

class Person {
  set firstName(value) {
    this._firstName = value;
    return value;
  }
}

or:

const person = {
  set firstName(value) {
    this._firstName = value;
    return value;
  }
}

or:

const Person = class {
  set firstName(value) {
    this._firstName = value;
    return value;
  }
}

or:

let person = {};
Object.defineProperty(person, "firstName", {
  set(value) {
    this._firstName = value;
    return false;
  }
});

Instead, we should remove the return statements from the examples above as follows:

class Person {
  set firstName(value) {
    this._firstName = value;
  }
}

or:

const person = {
  set firstName(value) {
    this._firstName = value;
  }
}

or:

const Person = class {
  set firstName(value) {
    this._firstName = value;
  }
}

or:

let person = {};
Object.defineProperty(person, "firstName", {
  set(value) {
    this._firstName = value;
    return false;
  }
});

Sparse Arrays

Empty slots in arrays are allowed in JavaScript. Array literals with only commas inside are valid. However, they may be confusing to developers.

For instance, the following arrays may be confusing to developers:

const items = [, ];
const strs = ["foo", , "bars"];

Instead, we should write the following:

const items = [];
const strs = ["foo", "bars", ];

We can have trailing commas.

Template Literal Placeholder Syntax in Regular Strings

Regular strings shouldn’t have template literal placeholders since they don’t work in regular strings and it’s easy for developers to mistaken them for template strings.

For instance, the following are probably mistakes:

"Hello ${name}!";
'Hello ${name}!';
"Sum: ${1 + 2}";

Instead, we should write:

`Hello ${name}!`;
`Sum: ${1 + 2}`;

Conclusion

There’re lots of characters that shouldn’t be in JavaScript code even though some may be allowed. Irregular whitespace characters shouldn’t be present in code. Characters made up of multiple code points shouldn’t in JavaScript strings.

Global objects shouldn’t be called directly. Only their properties are meant to be accessed.

Excess spaces in regex strings and literals, so they shouldn’t be included. Also, returning values in setters are useless, so they shouldn’t be added.

Multiple consecutive commas are allowed in JavaScript array literals, but they’re confusing to developers, so they probably should be avoided.