Is it a good practice to safeguard constructors for missing 'new'?

Question

From Secrets of the Javascript Ninja (great walkthrough btw):

// We need to make sure that the new operator is always used
function User(first, last){ 
  if ( !(this instanceof User) ) 
    return new User(first, last); 

  this.name = first + " " + last; 
} 

var name = "Resig"; 
var user = User("John", name); 

assert( user, "This was defined correctly, even if it was by mistake." ); 
assert( name == "Resig", "The right name was maintained." );

Do any real-world code style guides make all constructors do this kind of safeguarding (the first two lines)? I believe linters will catch calls like User("John", "Resig") and warn about the missing new, won't they?

Answer 1

I recommend you use strict mode and test in a browser that supports it. For example,

function User(first, last) {
    "use strict";
    this.name = first + " " + last;
}

this will throw in strict mode if you neglect the new. For example,

User("a", "b")

will throw a TypeError in Firefox. This allows you to catch errors without having to pay the penalty of an instanceof check. It throws a type error even if you call it outside of strict mode. This is because directly invoked strict mode funcitons have this bound to undefined instead of global. Setting a property on undefined results in a TypeError exception.

Answer 2

While checking your code on JSHint.com, it didn't tell me that I was missing new keyword:

function User(first, last){ 
  this.name = first + " " + last; 
} 

var name = "Resig"; 
var user = User("John", name); 

Result:

Line 2: this.name = first + " " + last;
Missing "use strict" statement.

Line 5: var name = "Resig";
Redefinition of 'name'.

---

But linter isn't the point here.

As for whether or not it is good practice, Yes it is a good practice because humans are not linters, they can forget to put new keyword. And even if a linter warns about it, I would still go ahead and safeguard my code.

There is nothing wrong in putting a simple; you could say extra condition that helps you avoid the problem.

There is a convention that first letter of a constructor function should be capital; that was introduced for the same reason so that a JS developer never forgets the new keyword when he says a function has first letter capital but not all developers may know about it (I didn't know until recently when I started exploring JS seriously), that's where you safeguard your code by putting it up yourself if not specified by outer world.

Answer 3

The code doens't warn, it just does call new if it wasn't used in the first place, so that calling User(x, y) will end up doing a new User(x, y) internally.

That said, I don't like this sort of transparent fixes. Defensive coding is okay (e.g. throwing an error), but I don't like code doing assumptions on what I want to do (or what I might have done wring, like using a capital letter by error when I wanted to call a function called user).

---

We (as in our company) do use the same "feature" of JavaScript though in the combination of a component and templating system we use internally.

Our "components" (JS object with behaviour attached to a specific DOM object) are created in this style: new MyComponent(domObject) - this creates the component and attaches it to the DOM element (where it can listen for events etc.).

We also use a templating engine which compiles to JS functions, so that you can just call the template as a function like this: myTemplate(someData) - this renders the given template to a HTML string. The templates can also call partials, which obviously are just normal functions returning HTML strings as well.

In order to directly generate the HTML for components, the component constructor will use the "detection" whether it was called via new or not to act as either a constructor or as template. When it works as template, it will add a special attribute to its root element (data-component in our case) so that a fixup can be done afterwards where the generated HTML is converted to a DOM tree and then all DOM elements with this attribute are automatically getting a component generated for them (using new this time).

This is just an example of how you can leverage such language constructs.

Answer 4

Just for fun: this could also be a way to safeguard against forgetting the new keyword:

function User(first,last,id){
  User.Instance = User.Instance|| function(firstname,lastname,id){
      this.firstname = firstname || 'nofirstname';
      this.lastname = lastname || 'nolastname';
      this.id = id || 0;
      if (!User.Instance.prototype.nameUpper){
       User.Instance.prototype.nameUpper = function(){
                   return this.name.toUpperCase();
       };
       User.Instance.prototype.resetId = function(){
                   this.id = 0; return this;
       };
       User.Instance.prototype.toString = function(){
           return [this.id,': ',
                   this.firstname[0].toUpperCase(),
                   this.firstname.slice(1),
                   ' ',
                   this.lastname[0].toUpperCase(),
                   this.lastname.slice(1)].join('');
       };
      }
  }
  return new User.Instance(first,last,id);
}
//usage
var pete = User('pete','johanssen',1),
    jean = User('jean','harlowe',2),
    mary = new User('mary','wsnovsky',3);
console.log(pete); //=> 1: Pete Johanssen'
console.log(mary); //=> 3: Mary Wsnovsky'

Concerning your question: I would consider 'Good Practice' quite normative/judgemental. Is it 'good practice' to eat a cows eye every day? If I were to answer that, I'd say no, but I'm sure not everyone in the world would agree to that. Using the pattern you demonstrated to circumvent a forgotten new keyword is not good or bad per se. It depends on the criteria you set: do you want to enforce a certain strictness, or do you want to safeguard against your own or your colleagues sloppyness? Will the pattern be less performant? Will the strictness from chukj's answer work in all situations, and if not, will that be sufficient for you?

One of the things I love and hate in javascript is its flexibility. JSLint, as far as I understood from the lectures of Douglas Crockford, is designed to force you to a uniform programming style - in his own words:

using jslint will hurt you

But on the other hand, jslint gives a lot of possibilities to ease that force using options like 'Tolerate misordered definitions', 'Tolerate uncapitalized constructors', etc. In other words, even using jslint, 'good practice' is not unambiguously defined.

Sometimes the javascript community behaves like 'the church of javascript denomination'. In it we find very strict believers, for whom the letter of law is the law, and every aberration to it is a cardinal sin (for whom may be "use strict" was specially invented ;). Others follow a more pragmatic path. There are regular churchgoers and people that never show up. Schisms emerge. Some are planning to leave this church and found a new one, e.g. where classes are allowed and considered good and sacred.

As may be clear by now: I think there is no such thing as 'good practice'. Or to phrase it in more pseudo-religious terms: good practice is in the eye of the beholder.