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);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?
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 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.
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.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.
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.(However, they do leave async exceptions from callbacks alone - only domains can handle those)
It is something like this:
---
---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.