Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Callback spaghetti is a sign that you're doing something wrong.

The first example from this page shows a request handler initialising a database connection and then executing a query. That's terrible separation!

Callbacks "spaghetti" actually does a great job of highlighting when you're not abstracting enough, any more than about 5 indents and you should be seriously considering refactoring your approach.

Thus

    app.get('/price', function(req, res) {
      db.openConnection('host', 12345, function(err, conn) {
        conn.query('select * from products where id=?', [req.param('product')], function(err, results) {
          conn.close();
          res.send(results[0]);
        });
      });
    });
becomes something like

    app.get('/price', function(req, res) {
      products.fetchOne(function(err, product) {
        res.render('product', { product: product });
      });
    });
Also, as other posts on this page mentioned, try{}catch{} is not how errors are handled in Node.JS, plenty of async operations will gain a fresh stack, and cannot be caught in this way.


  > Callback spaghetti is a sign that you're doing something wrong.
I have some Node.js code that does direct uploading via Amazon's S3 multipart uploading API -

  * multipart form processing, callbacks
  * each part requires multiple S3 API calls, callbacks 
  * parse XML results from the API, callbacks
Granted not all workflows are this complex. But many are - and they will result in callback hell. But saying that people are doing something "wrong" is at odds with the reality that complex workflows are a fact of life.


You misunderstand me, I think. Callback spaghetti is different from complex code that uses lots of callbacks.

I'm not saying you don't need to have a series of nested callbacks to do things, I'm saying you should hide these behind the appropriate abstractions for the task you're writing.

In your case the bullet points listed are exactly those layers of abstraction.

The request handler processes the form and calls the S3 layer for each file. The S3 layer then calls the APIs and passes off the responses to the XML parsing layer - which gives it back a useful JS object detailing the response, the API layer then provides a response to the request handler which formats the response and sends it to the client.

The workflow I'm implementing in Node is even more complicated than this, but good use of abstractions and control flow libraries[1] means that it's extremely rare for any code to be indented more than 5 blocks.

[1] https://github.com/caolan/async is my lib of choice, but plenty of other suitable choices exist.


becomes something like

You comfortably omitted the error handling (admittedly the original snippet didn't have that either).

However in production code it's not optional, and that's where node.js code tends to get really nasty.


On this point I don't disagree, for this case you'd expect the data access layer to deal with all the database errors and return something useful to the request layer:

    app.get('/price', function(req, res) {
      products.fetchOne(function(err, product) {
        if (err) {
          res.render('error', { error: err });
        } else {
          res.render('product', { product: product });
        }
      });
    });


you'd expect the data access layer to deal with all the database errors and return something useful

There you nailed the problem. It's a constant headache, especially since library authors have different ideas about the format and semantics of "something useful".

Before you know it you have re-invented your own half-baked exception-framework, to normalize/wrap/re-throw those ErrBacks. And then a week later you notice that rollback/retry and event cascades need a whole new level of treatment.

There's a reason why mature event-frameworks such as Twisted have never quite taken over the mainstream. It's sad to see node burn all its powder on a niche programming-style that just doesn't fly for the majority of applications.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: