11

I use something quite similar to the design pattern custom objects in my code normally.

But JSLint frowns upon constructs like this:

function MyClass() { this.init(); }
new MyClass(data);

Because the object is being discarded immediately after creation - it isn't being used for anything. We can fool JSLint to ignore this by assigning it to a variable, but it doesn't change that JSLint (and I am guessing many JavaScript enthusiasts) discourages the pattern.

So why is using side effects in a JavaScript constructor seen as a bad practice?

For what it's worth, I thought this was a good practice because:

  1. You have one setup function, thus it should be easier to maintain if e.g. you are managing a list of MyClass instances for access later. (Pushing an object onto an array is a side effect, you would have to do it after the constructor returned to be "good practice" = harder to maintain.)
  2. It has its own prototype, thus a "class ownership": Firebug reports this as an instance of MyClass instead of just Object. (This, in my opinion, makes it superior to the other design patterns.)
Aaron Kurtzhals
  • 2,036
  • 3
  • 17
  • 21
user1994380
  • 173
  • 6
  • JSLint discourages `new MyClass` because you're not using it after you've instantiated it. thus, it's only being used for its side-effects. Instead, this example could be rewritten to make use of dependency injection as `initialize(new MyClass());` (although this example is so simple as to be silly). – zzzzBov Feb 04 '13 at 20:33
  • Are you really never assigning newly created instances of `MyClass` to anything (in other words, is this a convoluted way to make `init()` a kind of static method)? "Normal" use of this pattern would not trigger JSLint warnings. – Frédéric Hamidi Feb 04 '13 at 20:34
  • @FrédéricHamidi not never, but sometimes it will have side effects which do the job itself, i.e. there's nothing more that needs to be done. A Person constructor might instantiate a person object, but you may not necessarily need to operate on that person right now. In the case you do need to, the side effects would register itself in an array for access later. – user1994380 Feb 04 '13 at 20:50
  • @zzzzBov JSLint's warning is rather misleading if side effects are not discouraged but using them for side effects exclusively is. The warning reads "Do not use 'new' for side effects." I may be mistaking but to me this implies that it must be used exclusively to set properties (or for the inheritance thereof). – user1994380 Feb 04 '13 at 20:56
  • A constructor can be used to perform functionality, it's just expected that you'll do more than simply instantiate a constructor. If all you're doing is instantiating an object and discarding it, you're better off using a normal function call. – zzzzBov Feb 04 '13 at 22:05

1 Answers1

11

In his book Clean Code, Robert Martin says

Side effects are lies. Your function promises to do one thing, but it also does other hidden things...they are devious and damaging mistruths that often result in strange temporal couplings and order dependencies.

What you described in your comment regarding arrays sounds like a "strange temporal coupling".

Aaron Kurtzhals
  • 2,036
  • 3
  • 17
  • 21
  • 1
    Helpful answer, thanks for search terms and reference material. I found this related for anyone with similar questions - [Mark Seemann's .NET blog](http://blog.ploeh.dk/2011/05/24/DesignSmellTemporalCoupling.aspx) (does not use JavaScript). Pending no better answers I will accept this one. – user1994380 Feb 04 '13 at 22:29