JavaScript Clean Code — Smells and Heuristics

John Au-Yeung - Jul 25 '20 - - Dev Community

Subscribe to my email list now at http://jauyeung.net/subscribe/

Follow me on Twitter at https://twitter.com/AuMayeung

Many more articles at https://medium.com/@hohanga

Even more articles at http://thewebdev.info/

Bad code has lots of unique characteristics. In this article, we’ll look at each one and what they are. We look at writing comments, functions, and general code smells and heuristics.

Comments

Inappropriate Information

Information that shouldn’t be in the comments like author and changelogs are in the comments. They should be in source control systems, bug trackers, and other record-keeping systems.

Change histories should be in source control systems for example. It has metadata for authors, code changes, change date, etc. These shouldn’t be in the comments.

Comments should be for technical notes about the code.

Obsolete Comment

Comments that are old, irrelevant, or wrong are misleading. They get old quickly. The code should be clean enough to not need so many comments.

They become outdated quickly, so they should be avoided.

Redundant Comment

If the code adequately explains itself, then we don’t need comments explaining it. JSDoc that says nothing more than the signature isn’t also very useful.

They should say things that can’t be shown by the code.

Poorly Written Comments

Comments that are worth writing should be written well. We should make sure that they’re the best comments that we can write.

Commented-Out Code

Commented out code can be misleading. Why are they still there if they’re commented out?

We should delete the code if they aren’t needed. They can also be reverted from the source control system’s change record.

Environment

Build Requiring More Than One Step

Builds shouldn’t require more than one step. The more things we have to do manually, the worse that everyone suffers.

We shouldn’t have to do manual operations like checking code out from source control or run a bunch of commands and scripts every time we have to run a build.

There’re so many build pipeline solutions that the button should be a one-click process.

Tests Requiring More Than One Step

Running tests should also be easy. All tests should be run with one command. Either we can run commands with one click on an IDE or by typing in one command.

Functions

Too Many Arguments

Functions should have a few arguments as possible. No arguments are best. More than 3 is questionable.

Output Arguments

We shouldn’t be returning arguments straight at the end of the function as-is. It just makes no sense.

Flag Arguments

Flag arguments mean that a function does more than one thing, so they should be eliminated.

Dead Function

Functions that aren’t called should be removed. Dead code takes up space and misleads people. We can always get it back from source control history.

General

Multiple Languages in One Source File

One file should only be one language. The more language is in a file, the more confusing it is.

Clean separation of logic and markup is always good. JSX is just a different syntax for JavaScript, so it’s actually one language.

Obvious Behavior That’s Unimplemented

Obvious behavior should be implemented. If it’s so useless that it can stay unimplemented, then we can probably eliminate it.

We should implement obvious functionality as described by a function so that the name isn’t misleading.

Incorrect Behavior at the Boundaries

Developers often trust their intuition when they write their functions and think that everything works. We often ignore corner and boundary cases.

We should check our code by writing tests for these conditions and don’t assume that it’ll work properly with them.

Overriding Safety Mechanism

Safety mechanisms in our code shouldn’t be overridden when we write code. Risky changes should be minimized. The end might be lots of bugs and lots of debugging.

Turning off failing tests are bad and we should think about the possible consequences when we do.

Duplication

Code duplication is bad. Every time we have to change duplicate code, we have to change them in multiple places.

We can remove them by abstracting the code and putting them in a central location.

Coding becomes faster and less error-prone since we only have to change things in one place.

The most obvious form is clumps of identical code.

Another form is conditional statements that appear multiple times in different parts of the code. We can replace them with polymorphic code.

Most design patterns are well-known ways to eliminate duplication. They’re structured to eliminate them.

Code at Wrong Level of Abstraction

When we abstract code, we should make them completely. The separation is complete. All the higher-level concepts to be in the base class.

Constants, variables, and utility functions shouldn’t be in the base class.

Source files, components, and modules should at a higher level of abstraction also be separated from ones with lower levels of abstraction.

We don’t wrong higher-level and lower-level code mixed together.

For example, if we have an Account class:

class Account {  
  saveAccountToDb() {}  
  getAccount() {}  
  setAccount() {}  
}

Then we have code at 2 levels of abstraction because we have saveAccountToDb which is a helper method to save data to the database.

We want to move it to a helper class or function.

Conclusion

Comments should be minimized and are useful and up to date when they’re there.

Functions should have as few arguments as possible and only do one thing. This also means we shouldn’t have flag arguments.

When we write code, we should check for boundary and corner cases to avoid bugs. Also, we should override safety features like removing important tests. They’re probably there for a reason.

Finally, code duplication is bad.

The post JavaScript Clean Code — Smells and Heuristics appeared first on The Web Dev.

. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .