0

I am a long time PHP developer who decided to give node.js and express a try today. I am trying to find the best way to merge my results into a single object. I may be approaching this to much like a PHP developer and could use some assistance. Thanks in advance.

app.get('/api/posts', function(req, res) {
    url_parts = url.parse(req.url, true)
    query = url_parts.query
    current_page = query.page || 1
    items_per_page = 50
    start_index = (current_page - 1) * items_per_page
    max_page = 1000

    var getPosts = {
        db: function() {
            var posts = {}
            connection.query('SELECT COUNT(*) AS count FROM rss', function(err, rows1, fields) {
                if (!err)
                {
                    total_pages = Math.ceil(rows1[0].count / items_per_page)

                    if (start_index < rows1[0].count || start_index < max_page)
                    {
                        sql = 'SELECT id, title, image, width, height, url FROM rss ORDER BY date DESC LIMIT '+start_index+', '+items_per_page
                        connection.query(sql, function(err, rows2) {
                            if (!err)
                            {
                                for (var i in rows2)
                                {
                                    comments = 'SELECT comment FROM comments WHERE section_id = '+rows2[i].id+' ORDER BY date DESC'
                                    connection.query(comments, function(err2, rows3) {
                                        //COMBINE RESULTS HERE
                                        //rows2[i].comments = rows3
                                    });
                                }

                                //res.json(rows2)
                               // DISPLAY RESULTS HERE
                            }
                            else
                            {
                                console.log(err)
                            }
                        });
                    }
                }
                else
                {
                    console.log(err)
                }
            });
        }
    }

    getPosts.db();
});

2 Answers2

1

Whatever you do, do NOT create loops with Node. That will simply create a blocking call. You need to use callbacks. You can do those manually or use a library that helps. I like asynch (https://github.com/caolan/async) so here is looping code replaced with async.

    async.forEachSeries(rows1, function(row, callback) {

       sql = 'SELECT id, title, image, width, height, url FROM rss ORDER BY date DESC LIMIT '+start_index+', '+ items_per_page
       connection.query(sql, function(err, rows2) {
           if (!err) {
               async.forEachSeries(rows2, function(row2, callback2) {
                      // do whatever processing you will do here.
                      // now call the callback2 to signal you have finished processing.
                      callback2();
               },function(err) {
                      // handle any errors that might occur in your 'loop' here.
               });
           }
    callback()
},function(err) {


});

Another suggestion for your code is that you should never build out your SQL manually. Use ? params. For example:

connection.query("SELECT id, title, image, width, height, url FROM rss ORDER BY date DESC LIMIT ?,?", [start_index, items_per_page], function(err, rows, fields) {
...do your processing
}

Hope this helps.

AlexGad
  • 6,612
  • 5
  • 33
  • 45
  • The advise on loops is bad. Use loops when you need them. Just make sure you don't iterate over millions of items or perform complex operations inside the loop. If you need to do that, consider using a queue/message system where you can perform that work in another worker process. – Felix Geisendörfer Jul 09 '12 at 15:11
1

AlexGrad is right about using the async module, it will help you organize your code better (and can also help with blocking the event loop, but that's very unlikely to be the case here). You should also follow his advise on using the ? character for escaping.

For your actual question, combining the results by adding them as ad-hoc properties is fine.

Felix Geisendörfer
  • 2,902
  • 5
  • 27
  • 36