4
\$\begingroup\$

I have this huge chunk of code. Is there any better to write this code, or am I too paranoid that I check every MongoDB query for error?

The logic is here:

  1. Bookcollection = collection of books
  2. Book = book info
  3. Query book collection of the user, if found find the book info for each of the user book collection
  4. Create a new collection and insert the book ISBN
  5. Send the ISBN to user

How can I improve this code, especially the error-handling?

BookCollection.findOne(newBookCollection, function(err, collection){
    if (err) {
        next(err);
    } else {
        if (collection) {
            collection.update({
                $inc: {
                    quantity: 1
                }
            }, function(err){
                if (err) {
                    next(err);
                } else {
                    Book.findOne({
                        isbn: collection.isbn
                    }, function(err, book){
                        res.json({
                            result: 1,
                            bookInfo: book
                        });
                    });
                }
            });
        } else {
            Book.findOne({
                isbn: isbn
            }, function(err, book){
                if (err) {
                    next(err);
                } else {
                    //Book not exist
                    if (!book) {
                        var googleBookApiUrl = provider.google.book + isbn + "&key=" + authKey.google.apiKey;

                        httpsRequest.get(googleBookApiUrl, function(err, data){
                            if (err) {
                                next(err);
                            } else {
                                if (data.totalItems > 0) {
                                    var volumeInfo = data.items[0].volumeInfo;
                                    var newBook = {};
                                    //Need to fix missing thumbnail
                                    var imageLinks = {};

                                    //Filter incoming result
                                    if (volumeInfo.hasOwnProperty('imageLinks')) {
                                        imageLinks = volumeInfo.imageLinks;
                                    } else {
                                        imageLinks = {
                                            smallThumbnail: "https://lh4.googleusercontent.com/-by3TNG1Dhtw/AAAAAAAAAAI/AAAAAAAAAN0/gOuqGurlI8o/photo.jpg?sz=64",
                                            thumbnail: "https://lh4.googleusercontent.com/-by3TNG1Dhtw/AAAAAAAAAAI/AAAAAAAAAN0/gOuqGurlI8o/photo.jpg?sz=64"
                                        };
                                    }

                                    newBook = {
                                        isbn: isbn,
                                        title: volumeInfo.title,
                                        subtitle: volumeInfo.subtitle,
                                        author: volumeInfo.authors,
                                        description: volumeInfo.description,
                                        contentVersion: volumeInfo.contentVersion,
                                        pageNumber: volumeInfo.pageCount,
                                        publisher: volumeInfo.publisher,
                                        publishDate: volumeInfo.publishedDate,
                                        categories: volumeInfo.categories,
                                        language: volumeInfo.language,
                                        printType: volumeInfo.printType,
                                        thumbnail: imageLinks
                                    };

                                    Book.create(newBook, function(err, book){
                                        if (err) {
                                            next(err);
                                        } else {
                                            BookCollection.create(newBookCollection, function(err){
                                                if (err) {
                                                    next(err);
                                                } else {
                                                    res.json({
                                                        result: 1,
                                                        bookInfo: book
                                                    });
                                                }
                                            });
                                        }
                                    });
                                } else {
                                    res.json({
                                        result: 0,
                                        err: 'Unknown ISBN code',
                                        bookInfo: null
                                    });
                                }
                            }
                        });                 
                    } else {
                        BookCollection.create(newBookCollection, function(err){
                            if (err) {
                                next(err);
                            } else {
                                res.json({
                                    result: 1,
                                    bookInfo: book
                                });
                            }
                        });
                    } 
                }
            });
        }
    }
});
\$\endgroup\$
  • 1
    \$\begingroup\$ Have a look at promises. You really want to mitigate that pyramid of doom. \$\endgroup\$ – Madara Uchiha Nov 29 '14 at 17:44
  • \$\begingroup\$ @MadaraUchiha even using promises, i could see there will be bunch of errors catching. no other ways to make this code better? \$\endgroup\$ – Tim Dec 1 '14 at 1:42
2
\$\begingroup\$
  1. Use Promises. Promises will help with all the indentation, and make your code more clean and understandable in the future (believe me, you'll have no idea of what you were trying to do if you read this code in a couple months)
  2. Break up your code in functions. Things like accessing Google Book API shouldn't be in the same place as retrieving data from MongoDB. There should be a clear separation between them; they should have names to help you quickly identify them. Objects are greatly fit for it: they allow you to name a bunch of code (i.e. a function) and call them as necessary.
  3. Make the functions return promises. This is extremely important, since it will allow you to create Promises chain.

Besides that, great job. :}

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Good job on your first answer. \$\endgroup\$ – SirPython Feb 13 '16 at 3:03
0
\$\begingroup\$

The $inc: { quantity: 1 } could probably be done in a async way without waiting for the result and so you can send the JSON faster.

Additionally, the create book and create collection could be done in parallel.

\$\endgroup\$
  • \$\begingroup\$ yup, we could do that, but still the code is huge anyway to simplify it? 2 problems in the code, too many if else for catching the error, and nested mongodb query \$\endgroup\$ – Tim Dec 1 '14 at 2:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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