Popular posts

Good question and several useful answers so far! Here are some of the bad practices I have encountered:

• Mixing early return with if-else

if (x) {
  return 1;
}
else {
  // Do something else
}

• Inconsistent naming e.g. variable name looks like a boolean, but you store something else

var isPublication = "publicationId";

• Making code harder to read and complicating it unnecessarily

return (x === 1 ? true : false);

• Lover of callbacks e.g. nesting too many callbacks when you can easily avoid them using something like async/await

dosomething(10, 12, (data) => {
  doSomethingElse(data, (something) => {
    doAnotherThing(something, () => {
      // More callbacks
    });
  });
});

• Making typos and copying the typos everywhere because you couldn't notice. Sometimes you misspell a variable name and copy-paste it to different places -- it works -- However, when other collaborators glance over the variable name and try to use it in some section, the code simply doesn't work. It wastes a lot of time because we are not able to figure out that the original variable name is incorrect.

• Failing to return early by mistake. Some developers accidentally skip return keyword while returning early in case of error.

if (err) {
  res.status(400).end(); // This doesn't halt the execution of code
}

// rest of the code

There are many more, but these are the ones I've come across most frequently. I have always highlighted them in the code reviews so far. :D