Hacker News new | ask | show | jobs
by isntitvacant 5553 days ago
re: cutting down on callback nesting / spaghetti code.

It can be hard to avoid; and having the community put together a PEP-8 style "best practices" to avoid creating nested code is one of my fondest dreams.

That said, I've written my fair share of [spaghetti code](https://github.com/chrisdickinson/tempisfugit/blob/master/li...) in Node, mostly because JavaScript seems to lend itself to slapping together nested callbacks without slapping you on the wrist for writing unreadable code.

In my experience, the best way to cut down on spaghetti code is to design your API around the [EventEmitter](http://nodejs.org/api/events.html#events.EventEmitter) object. Consider this adaptation of the linked spaghetti code: https://gist.github.com/f8533ff57844b1e54558

Note that in the application code, there are no nested callbacks -- they're merely concerned with validating the incoming data and passing the baton on to the next responder. Bonus points: the methods are in the order of their ideal execution. Clients using the code can just do the following:

    var validator = new RepoPathValidator(path);
    validator.on('data', function(repoData) {
        // do whatever needs to be done with the repo.
    });

    // redirect all errors to console.error
    validator.on('error', console.error.bind(console));
I'm very fond of using this method to destroy nested code -- I'm not sure how it's thought of in the community at large, though (part of the reason I'm so eager for the community to put together the aforementioned PEP-8 style "best practices" guide!)
1 comments
ehsanul 5553 days ago
I must be missing something, but how is EventEmitter helping you get rid of the nested code in your examples? It looks like changing all the anonymous callbacks to named functions, and then using the function names as the callbacks is what made the code more readable. EventEmitter wasn't necessary to do that, right?
link
isntitvacant 5553 days ago
Yes and no -- it's not absolutely necessary; I could've written it similarly without turning the callback-accepting functions into emitter-returning functions. It would have felt gross to do so, though. The downside I see with using straight callbacks that accept error and data arguments is that they conflate the logic of what that callback is actually attempting to do with the error-checking of the calling function. For example, my `baseDirOkay` method shouldn't care whether or not the calling `fs.stat` function succeeded or failed -- it should only care that the data it was passed is or isn't a directory. It shouldn't be called at all if the path was not valid. With an EventEmitter api, I can rely on the emitter itself to tell me whether or not to even call my `baseDirOkay` function, instead of having to conflate that logic with my method.

This seems like a tiny gain in readability, but it means that I don't have to make a decision between putting the error/success logic of `fs.readFile` in my method (where it does not go) and my calling function (where it probably does not go, and where it would incur unnecessary nesting -- and the finagling of the `this` variable that that incurs).

This may just be my taste -- but I find designing around (and reasoning about) EventEmitter-type API's much easier.

link