Avoiding using magic numbers in JavaScript - alternatives that work with JsHint

Question

JSHint's inspections now built into PhpStorm informed me about JavaScript magic numbers and I realise it'll make for clearer code to avoid using them.

I tried this:

var constants = {
    millisecs: 1000,
    secs: 60
};

and also this:

var constants = function () {
    this.millisecs = 1000;
    this.getMillisecs = function () {
        return this.millisecs;
    };
};

JsHint complains about both.

Taking the solution from this answer though works fine:

var constants = (function() {
    var millisecs = 1000,
        defaultMsgsPerSecond = 60;
    this.getMillisecs = function() { return millisecs; };
    this.getDefaultMsgsPerSecond = function() { return defaultMsgsPerSecond; };
})();

Presumably because of the closure. Why is it that this is accepted, whereas the other two suggestions taken from another SO question are not?

Edit: Although not triggering an error, it doesn't actually work. It errors to say constants is undefined. JsFiddle.

To clarify - by "works" I mean "doesn't trigger a warning from JsHint"

One problem clearly appearing with your first two code examples, but missing in the latter code example, is that the "constants" are not that constant - that is, can be changed later in the code (with `constants.millisecs = 100;`). — penartur, Aug 21 '12 at 09:28
@penartur that strikes me as being the answer - the working example immediately calls itself, making its properties private and thus only the two getters are exposed? — bcmcfc, Aug 21 '12 at 09:48

score 6 · Answer 1 · answered Jul 07 '14 at 17:33

In EcmaScript 6, you'll be able to just do:

const MILLISECS = 1000;
const DEFAULT_MSG_PER_SECOND = 60;

But until then, you can use EcmaScript 5's Object.freeze:

var constants = {
  millisecs: 1000,
  defaultMsgPerSecond: 60
};

var constants = Object.freeze(constants);

// Now constants is in fact... a constant!
constants.millisecs = 999;
constants.millisecs; // Still === 1000

And if it's your nature to be verbose, you can try Object.defineProperties:

var constants = {};

Object.defineProperties(constants, {
    'millisecs': {
        value: 1000,
        writable: false
     },
    'defaultMsgPerSecond': {
        value: 60,
        writable: false
     },
});

// Again, constants is in fact... a constant!
constants.millisecs = 999;
constants.millisecs; // Still === 1000

All 3 ways are good, but just using `const MILLISECS = 1000;` is easy and clean. I tend to use the third way to create things like a User object that I don't want anyone to be able to change. — Intervalia, Jan 22 '20 at 23:01

huysentruitw · Accepted Answer · 2013-10-30T20:01:22.630

about your edit

I think you wanted to new the inline object:

var constants = new (function() {
    var millisecs = 1000,
        defaultMsgsPerSecond = 60;
    this.getMillisecs = function() { return millisecs; };
    this.getDefaultMsgsPerSecond = function() { return defaultMsgsPerSecond; };
})();

But JSHint will also complain about that: Weird construction. Is 'new' unnecessary?.

If you use it as a closure, then you need to actually return something. If you don't, constants will indeed contain undefined. An easy fix would be to return this, but that would be a bad solution because you're extending this which is an instance of an object you do not own.

So returning an inline object seems to be the solution here:

var constants = (function() {
    var millisecs = 1000,
        defaultMsgsPerSecond = 60;
    return {
        getMillisecs: function() { return millisecs; }
        getDefaultMsgsPerSecond: function() { return defaultMsgsPerSecond; }
    };
})();

Avoiding using magic numbers in JavaScript - alternatives that work with JsHint

2 Answers2