@robinfeh

DEEP DIVE — Improve your JS code quality

Rob F.
6 min readDec 26, 2018

--

Recently I got assigned to review and refactor some internal projects. I encountered mistakes and decided to write about things we should know about Javascript in order to avoid bugs and increase the code quality.

While trying to rise to better code quality, we’ll go a bit deeper into some language details which hopefully aid to understand the claims.

The topics listed below are related to mistakes or imperfections which I’ve encountered during the reviews. I won’t unveil the mistakes which were made since I’m trying to help and not to make anyone feel bad.

If you really want to profit from this article, make sure to also have a look at the comments within the code snippets and the links below each topic to dive deeper and find further explanations.

let, const, var and their scopes

let and var are not interchangeable.

  • let per block scope, gets reassigned
  • const per block scope, never gets reassigned
  • var per function scope, might get reassigned

These days var is the weakest signal and therefore it should be avoided. A variable is only reassigned when we use the=character. Therefore functions such as Array.push() or Array.pop() don’t need let unless you clear the array by reassigning a new value.

Setting up ESLint in your project and editor, aids you to flag bugs, stylistic errors, and suspicious constructs in your source code and also can ensure that let is always reassigned.

Scopes in-depth:
When a function gets executed an ExecutionContext gets spawned and added to the stack which the engine processes. The let variables are defined within the LexicalEnvironment of the ExecutionContext. The var variables are defined within the VariableEnvironment of the ExecutionContext. As soon as the ExecutionContext is processed it can be flagged to be garbage collected.
https://stackoverflow.com/a/45788048/5242207

Links:
var, let, or const: https://link.medium.com/tcW5T7BKIS
clear arrays: http://2ality.com/2012/12/clear-array.html
ESLint: https://eslint.org/
GC: https://en.wikipedia.org/wiki/Garbage_collection_(computer_science)
ExecutionContext: https://stackoverflow.com/a/45788048/5242207

Modules are singletons

Since modules are singletons, please don’t declare variables within the module without remembering that this can introduce bugs easily. If you

require() or import

a module from two other modules, variables might be overwritten by one or the other. Rather define variables in factory functions or classes and return these from the module to avoid this problem.

rather use a construct like this to avoid this problem (factory function):

x now lives in the function’s scope instead of the module’s and is therefore initialized with the value 100 when the function gets invoked.

It can help to take a glimpse at transpiled modules to understand what’s happening under the hood. This example is produced by Webpack. https://gist.github.com/robinfehr/825885d6dc59c67c4521d438680d1a0cc

Links:
Singleton: https://www.sitepoint.com/javascript-design-patterns-singleton/
IIFE: https://en.wikipedia.org/wiki/Immediately_invoked_function_expres

Constructor Fn’s / Factory Fn’s / Classes

Let’s start with an overview over the three:

Constructor functions aren’t readable and cause this problems and always have to be instantiated with the new keyword, which would be ok but it is not enforced as it is using classes. Obviously, this can lead to problems.

Classes are syntax sugar for constructor functions but with the benefit that the new keyword is enforced and that methods are automatically not enumerable (e.g. not in for…in loops) hence they’re obviously the better choice. Since it’s just syntax sugar we can always refactor constructor functions to classes easily. Keep in mind that classes aren’t real classes since Javascript isn’t a class-based object-oriented language. It is a prototype-based object-oriented language. The extends keyword can be misleading. Multiple inheritance is no topic out of the box. To understand why we need to understand extends and the __proto__ chain. It is, of course, hackable if you’d want it very badly but... (follow the links below)

Let’s try to understand the extends keyword. It can be transpiled to:

Factory functions come in handy when you want to get rid of thethis and new keywords once and for all and use more functional programming paradigms which give you readable, concise, bug-free(er) code.

Encapsulation (data hiding) is achievable in both classes and functions. “Soft” (reflectable) and “hard” (non-reflectable) can be done in both (for the how follow the links). The syntax needed to achieve that in classes though can require getting used to. For encapsulation, I currently still prefer factory functions.

Btw. “hard” private fields in classes are now in stage 3 — (not available via babel atm.). Therefore, also encapsulation (data hiding) might soon be available out of the box. Since there clearly is a performance benefit using classes over factory functions when creating a lot of instances, we might even prefer them soon. Anyhow, I really hope that “hard” privacy will also be available for properties in objects generally. But more about that in another article.

  • for now — we should stick to factory functions or classes

Links:
__proto__ vs prototype:
https://stackoverflow.com/a/9959753/5242207
https://stackoverflow.com/a/11249437/5242207

Multiple inheritance:
https://stackoverflow.com/questions/29879267/es6-class-multiple-inheritance

Encapsulation in classes:
http://2ality.com/2016/01/private-data-classes.html

Understanding the private fields proposal:
https://github.com/tc39/proposal-private-fields/issues/93
https://github.com/tc39/proposal-class-fields/issues/178

Declarative and performance

You don’t need to program purely functional to write concise code. Good to know about the patterns used in FP such as Function composition — f(g(x)), Monoids, Monads, Functors etc. anyway. What I’m trying to say though is that we should write more declarative code. It reads so much faster.

  • Write reusable functions but don’t overdo it unless it’s necessary.

Remember — every time we enter a scope that contains function declarations, allocation happens for each function. For example, the prototype Object of the Function gets allocated and gets marked to be garbage collected as soon as the scope is left again (see the __proto_ vs. prototype links above). Keep such things in mind. Depending on your use case this might lead to problems when overusing e.g. factory functions or functional paradigms.

There are a lot of resources out there around FP, my starting point was this book: https://github.com/MostlyAdequate/mostly-adequate-guide

Links:
What is FP: https://link.medium.com/YPEpEyRDNS
Reduce GC activity: https://stackoverflow.com/a/18411275/5242207

Object.keys, Object.values, Object.entries

If you have never seen these or forgotten what these functions do:

These functions can help largely improve the readability and avoid bugs when for example the prototype got manipulated. For example, the .hasOwnProperty() checks can be avoided.

NPM and it’s package json
This topic might probably be a bit easier to digest but nevertheless, I wanted to mention it since we occasionally fail to remember.

shrinkwrapping your dependencies means creating an extra file that contains exact versions for each dependency and its transient dependencies.
This comes in handy when the versions within installed packages are installed using non-exact semver notations. We, therefore, can introduce bugs by accidentally installing a wrong version containing a bug or a breaking change.npm shrinkwrap can be used to create such a file with fixed versions.
It also includes dev dependencies. The file should be pushed to the repository since even your development environment can get broken by a transient module’s wrongly put semver notation. Shrinkwrapping can be greatly ignored if you have none or little dependencies whose transient dependencies semvers you manage to verify. You can play around with semver notations here: https://semver.npmjs.com/

publishing all modules is always important before we release the product.
To quickly test a module, it can be installed from Github via
npm install someAccount/someLibrary#someBranch
In the package.json that will look like:
“moduleName”: “git+ssh://git@bitbucket.org/someAccount/someModule.git“
But if we leave it like that, we don’t have semvers for these modules and therefore if a breaking change or a bug gets introduced in such, the consumers of our module will be affected immediately and therefore we could have a broken module.

the main file in the package.json should not be changed to another file than the index.js file since we’re all very used to looking for an index.js file and it’s just yet another unnecessary extra step to look up the main file if there is no index.js file.

Links:
Shrinkwrap: https://docs.npmjs.com/cli/shrinkwrap

I hope the article helped you to improve understanding some of the language concepts. If you liked it, don't forget to 👏 and follow me on twitter (@robinfeh).

@robinfeh

--

--

Rob F.

I like it reactive and colourful. Full stack dev.