Better pattern for accessing nested data in Javascript

Question

I am writing some code which iterates first through an array, and then further iterates through an array contained in the original array.

I am ending up with this weird pattern which I am repeating and I am certain is not optimized. While iterating through the last rssFeeds array item, it changes the value of 'triggerCallback' to true. Later, while iterating through the item array, a conditional checks if both triggerCallback is true and the items array is iterating through its last item, at which point it triggers a callback to be in used in async.js's waterfall pattern.

function countSuccessPosts(rssFeeds, cb){
  var successCounter = 0;

  var triggerCallback = ''

  rssFeeds.forEach(function(feed, index, array){
    if(index == array.length - 1){
      triggerCallback = 'true'
    }

    feed.itemsToPost.forEach(function(item, itemIndex, itemArray){
      if(item.response.id){
         ++successCounter

      }

      if(itemIndex == itemArray.length - 1 && triggerCallback == 'true'){
        cb(null, rssFeeds, successCounter)
      }
    })
  })

}

What's a more optimal way to structure this pattern?

Data Structure: RssFeeds will have up to 5 different itemsToPost elements.

[
  {
    "_id": "55808127b8f552c8157f74a7",
    "name": "",
    "imageUrl": "",
    "url": "http://www.taxheaven.gr/bibliothiki/soft/xml/soft_law.xml",
    "latestDate": "1434056400000",
    "endpoints": [
      {
        "_id": "554f9319bc479deb1757bd2e",
        "name": "Wise Individ",
        "id": 26413291125,
        "type": "Group",
        "__v": 0
      }
    ],
    "__v": 1,
    "itemsToPost": [
      {
        "title": "Aριθμ.: Υ194/12.6.2015 Τροποποίηση απόφασης ανάθεσης αρμοδιοτήτων στον Αναπληρωτή Υπουργό Οικονομικών Δημήτριο Μάρδα.",
        "summary": "Τροποποίηση απόφασης ανάθεσης αρμοδιοτήτων στον Αναπληρωτή Υπουργό Οικονομικών Δημήτριο Μάρδα.",
        "url": "http://www.taxheaven.gr/laws/circular/view/id/21113",
        "published_at": 1434056400000,
        "time_ago": "5 days ago",
        "guid": {
          "link": "http://www.taxheaven.gr/laws/circular/view/id/21113",
          "isPermaLink": "true"
        }
      }
    ]
  },
  {
    "_id": "558093013106203517f96d9c",
    "name": "",
    "imageUrl": "",
    "url": "http://www.taxheaven.gr/bibliothiki/soft/xml/soft_new.xml",
    "latestDate": "1434489601236",
    "endpoints": [],
    "__v": 0,
    "itemsToPost": [
      {
        "title": "Taxheaven - Άμεση ενημέρωση - Έγκαιρη επιστημονική κωδικοποίηση - Καινοτομικά εργαλεία. Κωδικοποιήθηκαν όλοι οι νόμοι στους οποίους επιφέρει αλλαγές ο νόμος 4330/2015",
        "summary": {},
        "url": "http://www.taxheaven.gr/news/news/view/id/24088",
        "published_at": 1434494400000,
        "time_ago": "about 4 hours ago",
        "guid": {
          "link": "http://www.taxheaven.gr/news/news/view/id/24088",
          "isPermaLink": "true"
        }
      }
    ]
  }
]

Answer 1

Wait, why are you using a callback when you can read the data synchronously?

After you've updated your question, it looks like you're just summing a number of elements in an array

Here's a fully synchronous version that counts the number of itemsToPost that have a valid response.id set.

function countSuccessPosts(rssFeeds) {
  return rssFeeds.reduce(function(sum, x) {
    return sum + x.itemsToPost.filter(function(y) {
      return !!y.response.id;
    }).length;
  }, 0);
}

If you're required to inject this into an async control flow, you can easily put a wrapper on it

function(rssFeeds, done) {
  done(null, rssFeeds, countSuccessPosts(rssFeeds));
}

The point tho, is that countSuccessPosts has a synchronous API because everything that happens within that function is synchronous.

Answer 2

You don't need to keep track of the last item. Just call the callback after both loops exit. I also changed the .forEach to for loops as these execute faster.

function countSuccessPosts(rssFeeds, cb) {
  var index, itemIndex, feed, item;

  for (index = 0; index < rssFeeds.length; index++) {
    feed = rssFeeds[index];

    for (itemIndex = 0; itemIndex < feed.itemsToPost.length; itemIndex++) {
      item = feed.itemsToPost[itemIndex];

      if(item.response && item.response.id) {
        successCounter++;
      }
    }
  }

  cb(null, rssFeeds, successCounter);
}

Of course, if you'd rather call countSuccessPosts without a callback the calling code can look like:

var successPosts = countSuccessPosts(rssFeeds);

And you can reformat the function to look like this:

function countSuccessPosts(rssFeeds) {
  var index, itemIndex, feed, item, successCounter = 0;

  for (index = 0; index < rssFeeds.length; index++) {
    feed = rssFeeds[index];

    for (itemIndex = 0; itemIndex < feed.itemsToPost.length; itemIndex++) {
      item = feed.itemsToPost[itemIndex];

      if(item.response && item.response.id) {
        successCounter++;
      }
    }
  }

  return successCounter;
}

Answer 3

I didn't check this but it is pretty similar to what I'm currently using in my project:

function countSuccessPosts(rssFeeds, cb){
  async.each(rssFeeds, function(eachFeed, outerCallback) {
     async(eachFeed.itemToPost, function(eachItem, innerCallback) {
        if(item.response.id) {
            //Do Something That Is Actually Async. Could be asking the server for success flag, for instance.
            doSomethingThatIsActuallyAsync(item.response.id).then(function (err) {
                if (!err) {
                    successCounter = successCounter + 1;
                }
                innerCallback();
            });
        } else { //This could be to skip "empty responses" without the need to go to the server, right?
            innerCallback();
        }
     }, outerCallback);
  }, function() {
      //All done
      cb(null, rssFeeds, successCounter);
  }); 
}

As others mentioned, you need this only if you have actual async methods calls inside the inner loop.