1

After a lot of googling I have not been able to confirm the correct approach to this problem. The following code runs as expected but I have a grave feeling that I am not approaching this in the correct way, and I am setting myself up for problems.

The following code is initiated by the main app.js file and is passed a location to start loading XML files from and processing into a mongoDB

exports.processProfiles = function(path) {

  var deferrer  = q.defer();
  
  q(dataService.deleteProfiles())  // simple mongodb call to empty the Profiles collection
    .then(function(deleteResult) {
      return loadFilenames(path); // method to load all filenames in the given path using fs
    })
    .then(function(filenames) {
      // now we have all the file names lets load and save
      filenames.forEach(function(filename) {
                  
        // Here is where i think the problem is!
        // kick off another promise chain for the dynamically sized array of files to process
        q(loadFileContent(path, filename)) // first we load the data in the file
          .then(function(inboundFile) {
            // then parse XML structure to my new shiny JSON structure
            // and ask Mongo to store it for me
              return dataService.createProfile(processProfileXML(filename, inboundFile));
          })
          .done(function(result) {
            console.log(result);
          })
        });
      })
      .catch(function(err) {
        deferrer.reject('Unable to Process Profile records : ' + err);
      })
      .done(function() {
        deferrer.resolve('Profile Processing Completed');
      });

   return deferrer.promise;
}

Whilst this code works these are my main concerns but cannot solve them on my own after a few hours of Google and reading.

1) Is this blocking? The read out to the console is difficult to understand if this is running asynchronously as i want it to - i think it is but advice on if I am doing something fundamentally wrong would be great

2) Is having a nested promise a bad idea, should I be linking it to the outter promise - I have tried but could not get anything to compile or run.

Autobat
  • 70
  • 8
  • First of all, avoid the [deferred antipattern](http://stackoverflow.com/q/23803743/1048572). – Bergi Jan 23 '16 at 22:40
  • @Bergi thank you, this article and the ones linked are excellent. Ensuring that I do not wrap something that is already a promise is key and I need to go and research Promisify. – Autobat Jan 23 '16 at 22:55

1 Answers1

2

I haven't used Q in a really long time, but I think that you'd need to do is let it know you're about to hand back an array of promises that need to all be satisfied before moving on.

Additionally as you're waiting for multiple promises on one section of code, rather than nesting further, throw the 'set' of promises back up once they're all satisfied.

q(dataService.deleteProfiles())  // simple mongodb call to empty the Profiles collection
.then(function (deleteResult) {
    return loadFilenames(path); // method to load all filenames in the given path using fs
})
.then(function (filenames) {
    return q.all(
        filenames.map(function (filename) {
            return q(loadFileContent(path, filename)) { /* Do stuff with your filenames */ });
        })
    );
.then(function (resultsOfLoadFileContentsPromises) {
    console.log('I did stuff with all the things');
)
.catch(function(err) {});

What you have is not 'blocking'. But really what you're doing with promises is moving things into a new 'block'ing section. The more blocks you have, the more async-ish your code will appear. If nothing else is running apart from this promise, it will still appear procedural.

But inner promises must still resolve before the parent promises resolve thereafter.

Inner promises like what you have aren't an inherently bad, personally I will break them out into seperate files to makes easier to reason about, but I wouldn't define that as 'bad' unless there's no need for that inner promise to exist, however where possible (and in your example here) I've adjusted so I throw back up the next set of promises for a new section to deal with the data after it's gotten it.

(I'm not great with Q though, this code will probably require a little further tweaking).

  • Thanks Darren, with your example and some of the links posted in the comments I think I am on the track to writing the properly... i think! – Autobat Jan 24 '16 at 00:02