Hacker News Reader

Comment by dkoch | Hacker News Reader

Another nitpick about the source code:

Let the code breath a little -- no whitespace around control structures is a bit jarring:

  if(error){
    ...
  }else{
    ...
  }

Instead:

  if (error) {
    ...
  } else {
    ...
  }

Maybe adopt this Node style guide: http://nodeguide.com/style.html

View on HN

Thanks for looking at the source! I'll give that change some consideration.

> Do not extend the prototypes of any objects, especially native ones. There is a special place in hell waiting for you if you don't obey this rule.

A strong statement whose motivation escapes me; it's not as though you can't trivially get the source of the extension in your REPL of choice.

Naming conflicts aside (what if your definition of "empty" doesn't match some other part of the code?), consider the `Array.prototype.empty` example. You extend `Array`, then some other bit of code anywhere else in the environment decides to do the following:

  var pets = ["dog", "cat", "rabbit", "turtle", "owl", "alligator"];
  for(var i in pets) {
    console.log(pets[i]);
  }

And what do you get?

  dog
  cat
  rabbit
  turtle
  owl
  alligator
  function () {
    return !this.length;
  }

Oops! And here you could argue that the other coder should've known to use hasOwnProperty or forEach or whatever, but, all that aside, extending native object prototypes is an easy way to break code all over the environment.

> And here you could argue that the other coder should've known to use hasOwnProperty or forEach or whatever, but, all that aside...

I don't think you can dismiss that argument so easily. Other people modifying natives is the reason that if you're unsure of your environment, you shouldn't rely on `for(el in array/object)` without appropriate `hasOwnProperty` checks.

If you have control over your environment and the developers who will work in/with it (ex. the code for your website vs an npm module you plan to publish), then I see nothing wrong with naked `for (el in array/object)` or with extending natives if it's worth the added cost of developer overhead (they have to know that they're working with extended natives).

This actually can be done safely with Object.defineProperty, using non-enumerable properties.

That doesn't work in IE8 (for non-DOM objects).

IE8 is the last version of IE for WinXP.

If you have to support IE 8, you probably shouldn't extend prototypes without serious thought, especially in a legacy codebase, because the interpreter support isn't there to do it right.

On the other hand, Windows XP is barely six months shy of EOL, and IE 8 currently has something like 4% share. Now would seem to be an excellent time to start dropping support for IE 8. Such support is never going to gain you more users, and many other browsers run on Windows XP.

"Don't use Object.defineProperty because a dying browser on a dying platform doesn't support it" strikes me as a rather weak argument. I mean, there are still people out there using IE 6, God help them. Why not shoot as low as we can?

The company I work for makes software for banks.

Neither WinXP nor IE8 are dead there, not by a long shot.

Naming conflicts aren't all that hard to resolve with a little forethought; if you're concerned about them, as you should be if you're thinking about extending prototypes in a codebase with which you're not intimately familiar, fire up your REPL and see what it returns for the name of the prototype extension you're thinking about adding. (If it's already there, that's one function you didn't have to write!)

As for the enumeration example, you're not making a point about extending the environment in general, but only about extending it wrong -- that is, by simply assigning functions to prototype slots, rather than using Object.defineProperty to do it right.

There are arguments to be made for writing style guides and programmer's cheatsheets with a half-competent audience in mind. All of those arguments are bad ones. The linked guide is generally reasonable, but I'd be a lot happier with it if it said "extending prototypes is usually a bad idea, but if you have to do it, here's how to stay off the landmines", rather than a superficially snicker-worthy insult to the reader such as "don't do this or you'll go to hell".

Reply on news.ycombinator.com