Avoiding using magic numbers in JavaScript - alternatives that work with JsHint
-
27-06-2021 - |
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"
La solution
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; }
};
})();
Autres conseils
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