B.James
B.James

Reputation: 196

Callback Hell and Refactoring

I have been learning JavaScript and started exploring Node. I have spent time understanding callback hell and how to fix it, but now I am more lost than ever and I'm starting to think I'm looking at it backwards (this might be a stupid question). I have read a guide on callbackhell.com and number 1 rule is to keep the code shallow.

Here is an example of connecting to Database, then reading a file, then inserting records to MongoDB and then logging:

MongoClient.connect(url, (err, db)=>{
   let dbo = db.db('test');
   fs.readFile('./data.json', (err, data)=>{
       let records = JSON.parse(data);
       dbo.collection('people').insertMany(records, (err, result) =>{
              console.log("result");
              db.close();
      })
   });
});

In the example, I have 3 anonymous callback functions and I have no idea how to refactor as the db is used throughout the callbacks. From what I understand I am should be aiming for 3 named functions (or am I missing something?) and call it like this:

MongoClient.connect(url, cb1);

and the functions something like this:

function cb1(err, db){
let dbo = db.db('test');
fs.readFile('./data.json', cb2);

}

function cb2(err, data){
    let records = JSON.parse(data);
    // ??? dbo.collection('people).insertMany(records, cb3)
    //now what?
}

function cb3(err, result){
    console.log(result);
    // ??? db.close?
}

Looking for any insight on Callback hell. Is this callback hell? How would I go about this? Am I missing something conceptually? Should it even be refactored?

PS. Promises and Async/Await can wait till I understand how to go about async programming using callbacks

Thank you!

Upvotes: 1

Views: 751

Answers (1)

trincot
trincot

Reputation: 350252

The nested callbacks are indeed what is commonly called callback hell.

Your attempt at separating the callbacks into named functions is fine, but you have realised one issue: that you need a reference to the db object in all callbacks, and it is not there.

You can solve this by either binding this to db, or else binding a (first) parameter for passing on that db object as argument. The principle is really the same.

Here is how it would look with binding this to the db object:

function cb1(err, db) {
    let dbo = db.db('test');
    fs.readFile('./data.json', cb2.bind(db));
}

function cb2(err, data) {
    let records = JSON.parse(data);
    this.collection('people').insertMany(records, cb3.bind(this));
}

function cb3(err, result) {
    console.log(result);
    this.close();
}

And here is how it would look with an extra parameter:

function cb1(err, db) {
    let dbo = db.db('test');
    fs.readFile('./data.json', cb2.bind(null, db));
}

function cb2(db, err, data) {
    let records = JSON.parse(data);
    db.collection('people').insertMany(records, cb3.bind(null, db));
}

function cb3(db, err, result) {
    console.log(result);
    db.close();
}

The next step would be to embrace promises and async await syntax. See for instance "How to use MongoDB with promises in Node.js?".

Upvotes: 1

Related Questions