Hacker News Reader

Comment by rdtsc | Hacker News Reader

Looking at it, in general, ignoring that I know about Node internals, you know what is the simplest way to solve this "problem"?

It is something like this:

---

    callback(find_largest(get_stats(read_files(dir)))

---

1 line.

If an error happens, an exception should be raised.

The fact that there are multiple such requests happening or that some of those operations have to wait for a select or epoll loop to return, should be handled by an underlying framework. The fact that even with promises you have to write 27 lines of code to do that, should be making you worried.

View on HN

Please stop spreading FUD.

With promises (Bluebird) this sounds like very little code in practice, assuming promisifyAll has been called the FS module:

- One line to read all files in a directory

- Map them with .map to a .stat call on them, returning array of promises.

- Reduce them to the largest element.

Something like:

    var fs = Promise.promisifyAll(require("fs"));
    var largest =  fs.readdirAsync("/your/file/path").map(function(file){
         return fs.statAsync(file);
    }).reduce(function(x,y){
        return (x.size > y.size) ? x : y;
    });

With arrow syntax:

    var largest = fs.readdirAsync("/your/file/path").
                  map((file) => fs.statAsync(file)).
                  reduce((x,y) => (x.size > y.size) ? x : y);

Alternatively, with Generators using Bluebird promises

     var largest = Promise.coroutine(function *largest(dirname){
        var files = yield fs.readdirAsync(dirname);
        var sizes = yield files.map(name => fs.statAsync(name));
        return files.reduce((x,y) => x.size > y.size ? x : y);
    });

If one error occurs, the promise rejects. VERY far from 27 lines of code. Not to mention you can of course compose these in one line if you extract them to methods like in your code:

    var largest = fs.readdirAsync("/your/file/path").map(fs.statAsync).reduce(largest);

> Please stop spreading FUD.

Was that addressed to strongloop.com? They are the ones that posted an example with promises using 27+/-whitespace lines of code and the initial problem algorithm only had 4 lines.

Sounds like you should write a blog post and not them about using promises.

I don't use Node.js on daily basis and am not familiar with promises so just commented as an outsider based on the article.

Your last one line looks fantastic and makes it clear what is going on. Any reason you can think of why that wasn't the "obvious" solution to the original posters of the article?

rtdsc: > Was that addressed to strongloop.com? They are the ones that posted an example with promises using 27+/-whitespace lines of code and the initial problem algorithm only had 4 lines.

Oh wow, this is my fault, I didn't read what they did there. I have __no idea__ why it took them 27 lines to implement this. It's rather trivial with promises.

- They have a .then followed by a .map there causing nesting for no reason whatsoever.

- They also get the file name, but handle this very poorly (with an array), where they could just do it as a part of the .map (the .all and .then are completely redundant) there

- They assume the possibility of directories, I didn't. That .filter is justified but that's the only thing one might have to add here.

- Generally, it's poor promise code.

I guess that promises are just not their tools of choice and they just used them _for that post_.

You may find StratifiedJS pleasing: http://onilabs.com/stratifiedjs

You wouldn't even need that one callback, just:

return find_largest(get_stats(read_files(dir)));

Exceptions are propagated as you would expect from synchronous code, despite being async under the hood.

The "promises" way of writing that concisely would be:

    read_files(dir).then(get_stats).then(find_largest).then(callback, errback)

Any exceptions in those functions would propagated to the "errback". Not perfect but much better than the examples in the article would suggest.

> If an error happens, an exception should be raised.

For better or worse, Node's best practice seems to be to propagate errors via the err parameter in each callback. E.g. `function myCallback(err, data)` where data represents the result of a successful computation.

One could reasonably debate whether this was the best possible design. Some people like exceptions, for one thing. For another, JS's dynamic typing, combined with multiple layers of error propagation, could lead to mishandling of the errors--you expect your callback to be passed error A, but it actually gets error B from elsewhere in the callback chain.

But this pattern is ingrained by now. If people started deviating from the pattern and using exceptions, you'd end up with an even worse monstrosity. Every bit of calling code would have to handle both styles of errors.

It's not just a best practice - exceptions raised from async code callback simply do not work right (they'll never make it back to the caller). In practice an exception raised from a callback will take a very short trip the wrong way through the stack (whatever happened to invoke your callback is _not_ the original caller, otherwise you wouldn't need callbacks), and end up as a toplevel uncaught exception, killing your entire process in nodejs.

Good point. I had previously thought the callback in some way "remembered" who called it, and could bubble an exception back up to the caller's try/catch block. But you've set me straight. Further proving the point that you can't just throw exceptions whenever you want.

Don't promises address this exception problem because the top level code is calling the then () function, which can then throw an exception to the appropriate caller?

Yes, I believe so. I was just addressing a the original suggestion, which was to avoid using promises and use vanilla callbacks instead. In that architecture, exceptions don't seem to be workable.

Actually, you can throw exceptions from within .then just fine and they will be always bubbled. You just cant go

  function g() {
    return somePromise.then(function() { 
      setTimeout(function() {
        throw new Error();
      }, 500);
    });
  }
  g(); // crash

But at that point, you're not using promises anymore so all bets are off.

However if you use a promise-based timeout too:

  function f() {
    return somePromise.then(function() {
      return Q.delay(1000).then(function() {
        throw new Error("Oops");
      });
    });
  }
  f().catch(function(e) {
    console.log(e.message); // logs Ooops.
  });

No problem bubbling that.

Promises normalize all exceptions in promise code to bubble up the chain - as such, they do make it back to the caller.

(However, they do leave async exceptions from callbacks alone - only domains can handle those)

It seems to me that Node.js is just manually writing in continuation-passing style, which is PAINFUL. Why not simply use a language which supports continuations natively? Heck, writing a Scheme->JavaScript compiler really shouldn't be that difficult.

Reply on news.ycombinator.com