Hacker News Reader

Comment by tantalor | Hacker News Reader

The revealing module pattern is an antipattern. Instead, just document which of your methods are private or protected,

  /** @private */
  MyClass.prototype.foo_ = function() {}

View on HN

I agree that this is an antipattern. We don't use it in Meteor. We did use it in a project that some of us wrote together several years before Meteor -- it was our first big JS project and we were heavily influenced by Crockford's _JavaScript: The Good Parts._, which appealed to our Lispy backgrounds. Our experience was that the added hassle in debugging wasn't worth the dubious benefit of locking rogue team members out of your private methods. And as this article notes, you have to go to significant lengths to preserve basic OOP conveniences such as `instanceof` and `this`.

If you're concerned about coworkers calling your private methods, the right tool is coding standards and code review. If you can't trust your coworkers to use good judgement even in the face of those things, you have bigger problem :) On the other hand, if your goal in using this pattern was to protect your private methods from bad people on the internet, you probably need to rethink your assumption that it is possible to sandbox JavaScript without interpreter-level support.

We do use (function () { ... })() in specific situations when we need to create a new scope, but this would typically only occur in framework code rather than application code (especially once the `linker` branch lands, which lets you declare specific exports and imports at the package level, and then the `meteor` asset build tool wires them up for you so that each package gets just the global environment it asked for.)

Also for now, we prefer to use prototypal inheritance directly rather than use a class library. On the latter, our perspective is that if Meteor developers want to use a class library, they should be able to pick their favorite rather than be stuck with ours.

I'm not convinced this is a good idea in JS, where the comment will be completely inaccurate. I'm not exactly sure how you'd differentiate something you decided was protected from something else you decided was private.

This isn't to say that having documentation on what you consider to be the API is a bad idea though. Omitting the functions you don't want or expect people to use is good for public facing docs.

I have to agree with you, if nothing else because existing debugging tools just aren't very good at dealing with the privacy-through-closures pattern. In many cases, it's essentially impossible to inspect the state of a variable that is made private this way, and it's not clear how you would even design a debugger that gave a reasonable interface for doing so.

Its annoying, but certainly possible to inspect the state of a local variable inside of a closure. Set a breakpoint inside of one of the exported functions, call it, and you have access to all the local variables within the closure.

I do dislike this pattern as I find the code is less structured, harder to debug, and less performant than attaching functions on the prototype. This outweighs the benefits of encapsulation to me.

I have never, ever come across a scenario where assigning to the object instead of the prototype has made any meaningful impact on performance.

The only scenario I can even think of where it would maybe possibly matter would be a tight loop where you are doing little else except just accessing properties.

I can imagine it having an impact on a graphics app where a `Point` class might be instantiated thousands of times, or even if the Backbone.Model constructor were hundreds of lines long to attach all the public methods. It would be weird to me to have to spend a few seconds of thought on a per-object basis to think how I should structure the code based on how often I think it will be instantiated.

But yes, I also have not been bitten by this. Its just something I've read about how the v8 engine works (and makes sense intuitively). I agree you shouldn't base coding style decisions soley on hearing one way is faster, but I think its a minor point and worth noting.

Edit: a link! https://news.ycombinator.com/item?id=2991904

That's true, but it requires that you have a side-effect free exported function for each scope. Adding such a debugger function is probably a smart move if you're using that pattern, but it is not standard practice, and as you said, it's still a hassle.

Reply on news.ycombinator.com