4 ms·

Another nitpick about the source code: Let the code breath a little -- no whitespace around control structures is a bit jarring: if(error){ ... }else{

by dkoch 13y ago

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 http://nodeguide.com/style.html

sintaxi 13y ago
Thanks for looking at the source! I'll give that change some consideration.
aaronem 13y ago
> 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.
- matchu 13y ago
  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.
  - lowboy 13y ago
    > 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).
  - taralx 13y ago
    This actually can be done safely with Object.defineProperty, using non-enumerable properties.
    - barrkel 13y ago
      That doesn't work in IE8 (for non-DOM objects). IE8 is the last version of IE for WinXP.
      - aaronem 13y ago
        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?
        barrkel 13y ago
        The company I work for makes software for banks. Neither WinXP nor IE8 are dead there, not by a long shot.
  - aaronem 13y ago
    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".

by dkoch 13y ago

sintaxi 13y ago

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

aaronem 13y ago

> 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.

matchu 13y ago
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.
- lowboy 13y ago
  > 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).
- taralx 13y ago
  This actually can be done safely with Object.defineProperty, using non-enumerable properties.
  - barrkel 13y ago
    That doesn't work in IE8 (for non-DOM objects). IE8 is the last version of IE for WinXP.
    - aaronem 13y ago
      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?
      - barrkel 13y ago
        The company I work for makes software for banks. Neither WinXP nor IE8 are dead there, not by a long shot.
- aaronem 13y ago
  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".