0

I'm trying to create a nodejs application using Goodreads API. When user search for books the app will fetch goodreads search result for the query and match with the local mongodb database for checking if the book information is already in local database. If not then the app will fetch every single book information and save them into local database for caching. My code is like this -

goodreads.searchBook(req.query.query, function (err, res) {
    if(err) next(err);

    var resultObj = JSON.parse(res);

    var books = resultObj.GoodreadsResponse.search[0].results[0].work;

    // console.log(books[0].best_book[0].id[0]._);
    // console.log(books.length);

    // show only 6 books
    var maxLoop = 6;
    if (maxLoop > books.length) {
        maxLoop = books.length;
    }



    var i;
    for (i=0; i<maxLoop; i++) 
    {
        var goodreads_id = books[i].best_book[0].id[0]._;
        Book.findOne({goodreads_id : goodreads_id}, function(err,  res){
            if (err) next(err);

            if(res) {

                console.log('book is already in datastore');

            } else {

                // ERROR : goodreads_id is always the last one 
                // OURPUT: example -
                // no book found in database for goodreads ID 5
                // no book found in database for goodreads ID 5
                // no book found in database for goodreads ID 5
                // no book found in database for goodreads ID 5
                // no book found in database for goodreads ID 5
                // no book found in database for goodreads ID 5
                console.log('no book found in database for goodreads ID ' + goodreads_id);
                goodreads.getBookInfo(goodreads_id, function(err, res){
                    if (err) next(err);

                    //console.log(res);

                    var resObj = JSON.parse(res);

                    var title = resObj.GoodreadsResponse.book[0].title[0];
                    console.log(title);
                });
            }
        });
    }
});   

I want to check every book ID to see if it is in database if not then get the single book info and insert into database using a for loop.

It should work but why always the last item is printing for the entire for loop?

Please help me to understand the problem. Thanks

UPDATE 1: the goodreads is a helper class I've created to contact with goodreads API. here is the code -

var http = require('http');
var xml2js = require('xml2js');


var parser = new xml2js.Parser();

// defining goodreads class

function goodreads() {
    this.url = 'http://www.goodreads.com/';
    this.key = '<DEVELOPER KEY>';
}

goodreads.prototype.searchBook = function (query, cb) {
    var requestUrl = this.url + 'search/index.xml?q=' + query + '&key=' + this.key;
    // console.log('sending query ' + query);
    http.get(requestUrl, function(res){
        var body = '';

        res.on('data', function (chunk) {
            body += chunk;
        });

        res.on('end', function () {
            parser.parseString(body, function(err, result){
                if(err) return cb(err);

                var jsonResult = JSON.stringify(result);
                return cb(null, jsonResult);

            });
        })
    }).on('error', function (err) {
        return cb(err);
    });
}

goodreads.prototype.getBookInfo = function(goodreadId, cb) {
    var requestUrl = this.url + 'book/show.xml?id=' + goodreadId + '&key=' + this.key;

    http.get(requestUrl, function(res){
        var body = '';

        res.on('data', function(chunk){
            body += chunk;
        });

        res.on('end', function(){
            parser.parseString(body, function(err, result){
                if (err) return cb(err);

                var jsonResult = JSON.stringify(result);
                return cb(null, jsonResult);
            });
        });

    }).on('error', function (err) {
        return cb(err);
    })
}

// take a book object with all information and update local database as needed
goodreads.prototype.updateLocal = function(book, cb) {
    // check if the book is already in local database

    // if it is in the local database then skip rest of the work and enjoy

    // If the book is not in local database then insert the book info to local databse

    // download and save the image using the book insert _id file name

    // insert author info into author database if its not already inserted
}

var GR = new goodreads();

module.exports = GR;

I'm learning nodejs and javascript for last couple of days. I am having trouble with async programming.

6
  • 1
    So what are you trying to do here really? Is it "find or create" given a list of data? What is goodreads and it's methods doing? Is this contacting some remote API? If so then what is the point? Do you need additional information here? You have a basic problem of calling async methods inside a loop that does not wait for completion of those method calls before iterating. However there are still many design an method usage flaws in here at first glance. Your question could benefit from explaining the reasons behind the questions asked here.
    – Neil Lunn
    Apr 7, 2016 at 8:03
  • I've edited the question with more explanation. I hope it will help @NeilLunn Apr 7, 2016 at 8:24
  • I really cannot see that you added much here other than what the goodreads methods are doing. The basic questions here were 1. Do you actually intend a "find or create" action? 2. What is the "point" of getting data from goodreads? Does it need to be used in the created data? Three questions I know, but the second point is really about those two things.
    – Neil Lunn
    Apr 7, 2016 at 8:32
  • Goodreads is my data source(like book title, description, isbn , rating etc) I am trying to see if any particular book's information is already in database, if not then insert those information in database for offline and fast access when needed. @NeilLunn Apr 7, 2016 at 8:39
  • What information needs to be inserted? This is what I keep asking over and over. Is it just the information in the array? Or are you adding data from the API? Asking you to be clear about this as your question and current code are not. So you are basically trying to "mirror", right? But we need to have some idea what data is actually intended to be "inserted".
    – Neil Lunn
    Apr 7, 2016 at 8:43

1 Answer 1

0

Book.findOne is asynchronous.

By the time you reach the callback function, even for the first book, your loop has already been completed.

To solve this, you can use promise-returning functions:

goodreads.searchBook(req.query.query, function (err, res) {
    if(err) next(err);

    var resultObj = JSON.parse(res);

    var books = resultObj.GoodreadsResponse.search[0].results[0].work;

    // console.log(books[0].best_book[0].id[0]._);
    // console.log(books.length);

    // show only 6 books
    var maxLoop = 6;
    if (maxLoop > books.length) {
        maxLoop = books.length;
    }

    // Promise-returning function
    var findBookPromise = function(goodreads_id){
        return Book.findOne({goodreads_id : goodreads_id}).then(function(res){
            if(res) {

                console.log('book is already in datastore');
                return Promise.resolve();

            } else {

                // ERROR : goodreads_id is always the last one
                // OURPUT: example -
                // no book found in database for goodreads ID 5
                // no book found in database for goodreads ID 5
                // no book found in database for goodreads ID 5
                // no book found in database for goodreads ID 5
                // no book found in database for goodreads ID 5
                // no book found in database for goodreads ID 5
                console.log('no book found in database for goodreads ID ' + goodreads_id);
                return goodreads.getBookInfo(goodreads_id).then(function(res){
                    //console.log(res);

                    var resObj = JSON.parse(res);

                    var title = resObj.GoodreadsResponse.book[0].title[0];
                    console.log(title);

                    return res;
                }, next);
            }
        }, next);
    };

    var promises = []; // Create an array to store the promises

    var i;
    for (i=0; i<maxLoop; i++)
    {
        var goodreads_id = books[i].best_book[0].id[0]._;
        promises.push(findBookPromise(goodreads_id));
    }

    Promise.all(promises).then(function(books){
        // Do something with the results
    });
});

Note: The above code is untested and may have error.

1
  • It has lots of problems, that much is for sure. Promises are not a "cure all", and just another method of doing things as opposed to using callbacks. So this still does not really address what the question asks.
    – Neil Lunn
    Apr 7, 2016 at 8:29

Your Answer

Reminder: Answers generated by Artificial Intelligence tools are not allowed on Stack Overflow. Learn more

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.