How you should not write code

There are many articles on the web on how to write better and clean code. I had emphasized the words clean and better. I think by the time all of us had adopted those concepts in day to day coding life. However, sometimes I feel that we do over-engineering and copy-paste things. It is to save time. In this article, I will highlight some of the code practices/smells that we should either drop or don't follow it.

This is an article from a mini-series of How you should not write code. If you know how to write code and want to write better code. Please checkout work of genius Robert Martin.

1. Dependency Injection: I have seen many libraries that have created a dependency injection. DI makes code more readable and maintainable. However, this is not a valid case for JavaScript. JavaScript is a dynamic language. You can modify the context of the object or constructor on runtime.

Let's take the Client/Service DI example of Wikipedia.

class Service {
  getName() {
    return "This is service";
  }
}
module.exports = Service;

Create a Client Class with and without DI

const Service = require("./Service");
class ClientWithoutInjection {
  constructor() {
    this.service = new Service();
  }
  greet() {
    return "Hello " + this.service.getName();
  }
}
class ClientWithInjection {
  constructor(service) {
    this.service = service;
  }
  greet() {
    console.log(this.service.getName);
    return "Hello " + this.service.getName();
  }
}
exports.ClientWithInjection = ClientWithInjection;
exports.ClientWithoutInjection = ClientWithoutInjection;

Let’s write some unit test cases: I am using jest here.

const {
  ClientWithInjection,
  ClientWithoutInjection
} = require('./Client')
const Service = require('./Service')
describe('DI', function () {
  describe('Client', function () {
    it('check it', function () {
      const cw = new ClientWithInjection(new Service())
      const co = new ClientWithoutInjection()
      expect(cw.greet()).toBe(co.greet())
    });
  });
});

In the above test suite, I have satisfied both the class here. Nothing fancy. All the tests work fine. **_cw.greet()_** and **_co.greet()_** return the same value which is “Hello This is service”.

Now let’s write another test file with some magic of mocking

jest.mock('./Service', () => (function () {
  this.getName = () => "something else"
}));
const {
  ClientWithoutInjection
} = require('./Client')
describe('DI', function () {
  describe('Client', function () {
    it('check it', function () {
      const co = new ClientWithoutInjection()
      expect(co.greet()).toBe("Hello something else")
    });
  });
});

If you notice, I have to change the expected value in the test case. This will still work. This is because I have mocked the service to return “something else”. So we can stop overthinking DI at least for Javascript.

2. Underscore(_) and $ for private variable: **_/$** for private variable become famous from the ages when people were coding in languages like C++, C. At that time we didn't have any coding guidelines and linting-tool to prevent bugs. And it becomes naming conventions. I have seen variable names like ___name__something__new. It’s like adding more _ and the variable will become more private. By the time, things had been changed. Now we have superb IDE and plugins to support.

If you really care about the private variable. You can use a Symbol with ES2015 or the latest. Babel/Typescript also supports private variables. Below are a few samples to demonstrate.

Using Symbol:

const property = Symbol();
class Something {
  constructor() {
    this[property] = "test";
  }
}
var instance = new Something();
console.log(instance["property"]); // undefined
console.log(instance); // Something { [Symbol()]: 'test' }

Using old-style constructor/closure:

class Person {
  constructor(nm) {
    this.setName = function (name) {
      nm = name;
    };
    this.getName = function () {
      return nm;
    };
  }
}
const person = new Person("deepak");
console.log(person.getName()); // "deepak"
person.setName("kapeed");
console.log(person.getName()); // "kapeed"
function PersonF(nm) {
  this.setName = function (name) {
    nm = name;
  };
  this.getName = function () {
    return nm;
  };
}
const personf = new PersonF("deepak");
console.log(personf.getName()); // "deepak"
personf.setName("kapeed");
console.log(personf.getName()); // "kapeed"

3. Explicitly assign null to a variable: null is not undefined, and null is not false. When I say this statement, I mean it. Many of the times when I get a code to review, often I see people assign explicitly null to a member variable, to signify that it contains no value. However, this could lead to a big issue and can become a code smell. See the below examples.

class Person {
  constructor(name) {
    this.name = name || null;
  }
}
var deepak = new Person();
// how they check
if (deepak.name == null) {
  console.log("null")
}
console.log(deepak.name && true); // null
deepak = new Person("");
console.log(deepak.name && true); // null
deepak = new Person(false);
console.log(deepak.name && true); // null
deepak = new Person(0);
console.log(deepak.name && true); // null

A better way of writing the same code using Symbol

// Better option
const NO_NAME = Symbol();
class BetterPerson {
  constructor(name) {
    this.name = name || NO_NAME;
  }
  hasName() {
    return this.name !== NO_NAME;
  }
}
deepak = new BetterPerson(0);
console.log(deepak.hasName()); // false
deepak = new BetterPerson("deepak");
console.log(deepak.hasName()); // true

Image for post

4. Basic use of iterator methods[forEach, map, reduce, etc.]: Lots of the time I have seen that people are still using traditional for loop. Maybe they want to write super-fast code or they trust themselves a lot. But using for loop of your own (using array.push) could lead to code smell. JavaScript has such good support for the array. At the same time is simple, we should leverage the power of Array methods.

// Program:: remove all odds and multiply 5, so that u get all numbers divisible by 10
//Data set
let data = [1, 2, 3, 4, 5];
const MULTIPLIER = 5;
//Data set
//Example 1
let mapped = [];
for (let index = 0, len = data.length; index < len; index++) {
  if (data[index] % 2 == 0) mapped.push(data[index] * MULTIPLIER);
}
console.log(mapped);

With array reduce:

//Example 2
mapped = data.reduce((curr, next) => {
  if (next % 2 == 0) curr = [...curr, next * MULTIPLIER];
  return curr;
}, []);
console.log(mapped);

You can see in Example 1, There are too many things to take care of. In example 2, it looks complex. However, if you break down the example then it is easy to understand.

//Break down
mapped = [];
const isEven = num => num % 2 == 0; // Function returns if even
// Accumulator fun to collect all even number
const accumulator = (records, num) => { 
  // push the current numb at last and create new array
  if (isEven(num)) {
    records = [...records, num * MULTIPLIER];
  }
  return records;
};
mapped = data.reduce(accumulator, []);
console.log(mapped);

Image for post

5. Try-Catch Hell: Whenever you write async-task and you expect an error. In the case of Error/Fail, it becomes spaghetti code. It is very hard to maintain the catch block. Let’s see by example.

const delay = () =>
  new Promise(r => {
    setTimeout(r, 1000);
  });
const fetchData = async function(url) {
  await delay();
  if (url == "fail") throw "CODE_FAIL";
  if (url == "fatal") throw "CODE_FATAL";
  if (url == "") return "OK";
};
async function main() {
  try {
    const data = await fetchData("fail");
  } catch (e) {
    if (e == "CODE_FAIL") {
      // Show toast message
      console.log(e); // CODE_FAIL
    }
    if (e == "CODE_FATAL") {
      // Show popup message
      console.log(e);
    }
  }
}
main();

Here all business logic of the failure is written inside the try-catch. This is ok for small code. However, you have the same logic in multiple places. Then you can simplify catching in the call .catch method and reuse function.

// catch function
const onCatchError = e => {
  if (e == "CODE_FAIL") {
    // Show toast message
    console.log(e);
  }
  if (e == "CODE_FATAL") {
    // Show popup message
    console.log(e);
  }
};
const onSomeOtherCatchError = () => {};
async function mainNew() {
  try {
    const data = await fetchData("fail")
      .catch(onCatchError) // CODE_FAIL
      .catch(onSomeOtherCatchError);
  } catch (e) {
    console.log("Something else handle here");
  }
}
mainNew();

6. A long path for import directory: I have seen path and directory structure nested to 5–10 levels. Once you have such a directory structure, you see long import paths. As a human being, it is hard to navigate backwards to find the correct file. However, Reverse(Traversing from root to file) is easy. See example:

// nest/nest/nest/nest/nest/nest/nest.js
const Service = require('../../../../../../dependency_injection/client/Service')console.log(new Service().getName())
// package.json
"scripts": {
  "nestWithout": "node nest/nest/nest/nest/nest/nest/nest.js",
  "test": "jest"
},

The required service path is too long. If you have the privilege of Webpack, I will recommend you check Webpack resolve. Else you can apply a basic solution.

By default, NODE_PATH points to the global node_modules path. You can add any directory in $_NODE_PATH_ environment variable. Once the user adds any folder in NODE_PATH. It merges with the existing path. And you can require a file as node_module relative to the entry point.

// package.json
"scripts": {
    "nest": "NODE_PATH=./ node nest/nest/nest/nest/nest/nest/nest.js",
    "nestWithout": "node nest/nest/nest/nest/nest/nest/nest.js",
    "test": "jest"
  },


// nest/nest/nest/nest/nest/nest/nest.js
const Service2 = require('dependency_injection/client/Service')
console.log(new Service2().getName())

I will try to add more examples in the future. All the source code can be found on GitHub or Gist. Please do let me know how you code.

How you should not write code | JavaScript

Comments (1)

More from this blog

5 Patterns to Write Better and Clean Code in Python

Validation in Python using Metaprogramming and Decorator- Advanced Python

Infinite scroll | Pagination on API using JavaScript Generators

How to work with Date | JavaScript Weird Parts

Internationalization in Plain React.js

Command Palette

Comments (1)

More from this blog