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"

Était-ce utile?

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
Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top