skip to Main Content

I’ve been tasked with refactoring/simplifying the architecture of a (relatively) large node.js codebase and it makes ample use of global variables. I don’t have much experience with javascript so just wanted to get a general sense of whether this is indeed super confusing and bad practise or if I’m in the wrong.

Here’s a dummy example of a pattern I see throughout the codebase:

eg. 3 files: A.js, B.js, C.js:

A.js:

A_init = () => {
    // sets global variable a
    a = 1
}

exports.A_init = A_init

B.js:

B_init = () => {
    // sets global variable b
    b = 2
}

exports.B_init = B_init

C.js:

C_init = () => {
    // sets global variable c
    // depends on global variables a and b being set
    c = a + b
}

exports.C_init = C_init

In reality there are many more files in the library (around 40) and some are thousands of lines long with random functions spread throughout the code which set variables at a global level which other library files have methods to utilise.

So for eg. if a developer wishes to utilise the library, they might write something like this:

UserScript.js

const A = require("./A");
const B = require("./B");
const C = require("./C");

A.A_init()
C.C_init()
B.B_init()

someMethod = () => console.log(a, b, c) // expected: 1, 2, 3 actual: ReferenceError: b is not defined

My experience of using the codebase is these sorts of ReferenceErrors surface very frequetly due to people either not knowing they need to call certain functions in order to set global variables other functions depend on or calling them in the wrong sequence.

There are probably around 50-100 such global variables and it’s a nightmare to try and extract out… If this question is too vague I appologise, but what strategies would you suggest I adopt to address this? Is some use of global variables like this acceptable? Are there standards people adopt to constrain their usage? Would adopting a more OO approach and using classes/constructors be something you would recommend here?

I have tried sourcing for answers online but I could not get any. I hope to find answers here.

2

Answers


  1. In general you need to avoid using global variables (50-100 is definitely a lot) as they may be overwritten by other functions (specially that JavaScript is an asynchronous and concurrent programming language), and overusing it makes the code non-readable.

    JavaScript supports a wide range of programming patterns such as OOP, Functional programming, dependency injection, etc.

    To get more information on Javascript best practices visit the following link:
    https://www.w3.org/wiki/JavaScript_best_practices

    Based on your exact needs, maybe you could replace these bad patterns with patterns like modular-programming, object-oriented-programming, dependency injection, etc.

    A last word:
    Programming languages are created to help us (programmers) understand the machine language. so, more a code is readable (not confusing), better that code is.
    On the other side program codes finally get compiled or interpreted, and executed by a computer. so its also important that the code follows some standards (like SOLID principles for OOP) to avoid bugs (specially non-deterministic bugs), and performance issues.

    Please let me know if you have any questions.

    Login or Signup to reply.
  2. Caleb.

    • Start with writing regression tests, so you can be confident not to
      break too much when you refactor.
    • Find out where and how certain variables are used. Add comments. Give
      those global variables better names, if they don’t have already, and
      make sure the same variable isn’t used for different purposes.
    • Then start replacing the use of global variables by explicit
      parameter passing and explicit return values. That will reduce the
      temporal coupling issues you showed in the question.
    • If you now notice some of your functions becoming too many
      parameters, identify parameters which are always passed as a group,
      make a data structure out of them, so you can pass them together.
      Make sure that data structure stays a useful abstraction.
    • If you find certain operations which work mainly on those data
      structures alone, convert the data structure into a class and
      implement the operations as member functions.
    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search