0

I have a sticky problem I am trying to solve. To illustrate the problem, I will use a familiar scenario: traversing a directory. I know there are tons of libraries out that that already traverse a directory. However, that's not what I'm trying to do. Traversing a directory is just a metaphor for my problem.

Basically, I have the following:

structure: [],

traverseDirectory: function(path) {
  var scope = this;

  var promise = new Promise(function(resolve, reject) {
    openDirectory(path, function(results) {
      for (var i=0; i<results.length; i++) {
        if (results[i].type === 'directory') {
          scope.traverseDirectory(results[i].name);
        } else {
          scope.structure.push({ filename:name });
        }
      }
      resolve(scope.structure);
    });
  });
  return promise;
},

getDirectoryStructure: function(path) {
  this.traverseDirectory(path)
    .then(function(results) {
      // Print out all of the files found in the directories.
      console.log(JSON.stringify(results));
    }
  ;
}

My problem is the .then of getDirectoryStructure fires before the directory is actually traversed. Its not waiting like I thought it would. Plus, I'm not sure how to "pass" (not sure if that's the right word) the promise around as I'm recursing through the directory structure. Can I even do what I'm trying with promises?

Thank you for any help.

JQuery Mobile
  • 6,221
  • 24
  • 81
  • 134
  • I kind of miss the goal. do you search for item, or do you want to build some kind of representation of the structure? That would make significant difference in approach. – webduvet Oct 29 '15 at 14:18

3 Answers3

1

In this case you would need to consider that you have multiple "steps" per level... or in your directory traversal example multiple sub directories, so essentially you need to fork... what @Anonymous0day suggests is close, however returning out of the for loop is counter indicative.

What you need is Promise.all: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all

var traverseDirectory = function(path) {

  var promise = new Promise(function(resolve, reject) {
    openDirectory(path, function(results) {
      resolve(results);
    });
  });
  return promise.then(function(results) {
    var pros = [];

    for (var i=0; i<results.length; i++) {
      if (results[i].type === 'directory') {
        pros.push(scope.traverseDirectory(results[i].name)); // <- recursive call
      } else {
        pros.push([{filename:name}]);
      }
    }

    return Promise.all(pros).then(function(arrs) {
       var structure = [];
       for (var i=0; i<arrs.length; i++)
         structure = structure.concat(arr[i]);
       return structure;
    });
  });
}

(PS I kinda de "scoped" this, to show you that you don't need an external object in the same way to keep track of the structure... you can keep it inside the function and only expose it when the outer promise resolves).

But the biggest thing you needed to do was actually WAIT to resolve the outer promise until after you're all done traversing (PS - I'll leave it to you to see what Promise.all does if the 'pros' array is empty).

You were seeing it execute immediately because it was literally resolving right after it was done with the for loop... if those recursions had actually been asynch, the event loop would indeed immediately resolve.

Cheers, lemme know if that makes sense. [EDITED for proper Promise.all().then(success, fail) instead of the .catch I had].

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
Nick Sharp
  • 1,881
  • 12
  • 16
  • PS- I think I may have buggered up the then.catch on the Promise.all... You might need it more like this: Promise.all([p1, p2, p3, p4, p5]).then(function(value) { console.log(value); }, function(err) { console.log(reason) }); – Nick Sharp Oct 29 '15 at 02:27
  • Avoid the [`Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572)! You should only promisify `openDirectory` and do everything else via `then`. – Bergi Oct 29 '15 at 10:30
  • @NickSharp - I had an external scope because I need to see if a file had already been added. In the scenario presented, a file wouldn't be added twice because it involves directory traversing. However, traversing tags is more representative of my scenario. Sp, I need to check to see if a file has already been added before I add it. Which leads me back to my scoping issue. Right now, files are getting added multiple times to the `structure[]`. I'm not sure how to resolve this. Also, what's the Promise constructor anti pattern? I read the link. It doesn't make sense to me in this situation. – JQuery Mobile Oct 29 '15 at 13:33
  • @Bergi How would you fix this answer to avoid the Promise constructor anti pattern? I read the link. I've been pondering it. Unfortunately, it doesn't make entire sense to me. – JQuery Mobile Oct 29 '15 at 13:34
  • @JQueryMobile: See my edit to the answer (I've also fixed the `structure` thing to include the results from the recursive calls) – Bergi Oct 29 '15 at 14:00
  • @Bergi - Thank you for your response. However, I'm still not getting the directory structure from my caller. Its like we keep going down the directory structure, but never return the results. When `getDirectoryStructure` gets called, the results are not getting printed. However, if I have `console.log` statements in the `traverseDirectory` function, I can see its getting traversed. Why aren't the results getting back to `getDirectoryStructure` after the traversal is complete? – JQuery Mobile Oct 30 '15 at 12:58
  • The code in this answer should get the results back. What results are you seeing instead, empty arrays? Are you sure you're not missing one of the `return`s? – Bergi Oct 30 '15 at 13:02
  • @Bergi - I just don't see any results. Nothing is printed to the console window. Its like it goes down in the stack, but never comes back up the stack. – JQuery Mobile Oct 30 '15 at 14:19
  • @Bergi - The line(s) that do not make sense to me is the `var pros = [];`. I assume `pros` is an array of Promises. However, it does not make sense to me why `scope.traverseDirectory(results[i].name)` or `[{filename:name}]` is pushed. Is it that `pros` isn't an array of promises. Instead, it should be an array of files? – JQuery Mobile Oct 30 '15 at 14:34
  • @JQueryMobile: Yes, `pros` is supposed to be an array of promises. `scope.traverseDirectory(results[i].name)` returns a promise for an array. `[{filename:name}]` is actually not a promise but `Promise.all` will will treat it like a promise for that array just as well. If you like it explicit, you can write `push(Promise.resolve([{filename:name}]))` – Bergi Oct 30 '15 at 14:43
  • Hm, not seeing any results is odd. Either there's a promise that never resolves (`openDirectory` never calls back?), or there is an exception somewhere so the promise (chain) is rejected. Try to add an error handler in `getDirectoryStructure` as well. – Bergi Oct 30 '15 at 14:45
  • Wow. Adding a `catch` handler totally fixed the problem. Thank you for sharing your wisdom. – JQuery Mobile Nov 02 '15 at 15:18
1

Here is a relatively concise way to do it that avoids for-loops and mutating variables. It returns a tree structure of all the retrieved results:

// returns a promise for an object with two properties:
//   directoryname (string)
//   contents (array of objects (directories and files) for the contents of the directory)
function traverseDirectory(path) {
    return new Promise(function(resolve, reject) {
        openDirectory(path, resolve);
    }).then(function (results) {
         return Promise.all(results.map(function (item) {
             return item.type === 'directory'
                 ? traverseDirectory(item.name)
                 : { filename: item.name };
        }));
    }).then(function (contents) {
       return {
           directoryname: path,
           contents: contents
       };
    });
}

If your objective is to obtain a flat array of all files in the directory tree, you can do this (everything is the same except for the last then):

// returns a promise for an array of all of the files in the directory and its descendants
function traverseDirectory(path) {
    return new Promise(function(resolve, reject) {
        openDirectory(path, resolve);
    }).then(function (results) {
        return Promise.all(results.map(function (item) {
            return item.type === 'directory'
                ? traverseDirectory(item.name)
                : { filename: item.name };
        }));
    }).then(function (contents) {
        return Array.prototype.concat.apply([], contents);
    });
}
JLRishe
  • 99,490
  • 19
  • 131
  • 169
1

For a more flexible approach, you might choose for :

  • .getDirectoryStructure() to deliver what it says, a representation of a directory hierarchy.
  • A flattener to operate on the hierarchical array to produce what is actually asked for - a flat array of objects. You can use something like lodash's _.flattenDeep() or write your own.

First, a couple of general points :

  • In general, you should promisify at the lowest possible level. In this case, that means promisifying a version of openDirectory(). By doing so the code in traverseDirectory() will simplify. (In the solution below, what remains of traverseDirectory() is actually subsumed by getDirectoryStructure()).
  • Use Promise.all() to aggregate families of promises generated during the traversal.

In doing this, you can dispense with outer var structure and rely instead on Promise.all() to deliver, (recursively) an array of results.

Here's the code :

var dirOpener = {
    openDirectoryAsync: function(path) {
        return new Promise(function(resolve, reject) {
            openDirectory(path, resolve);
        });
    },
    getDirectoryStructure: function(path) {
        var scope = this;
        return scope.openDirectoryAsync(path).then(function(results) {
            var promises = results.map(function(file) {
                return (file.type === 'directory') ? scope.getDirectoryStructure(file.name) : { filename: file.name };
            });
            return Promise.all(promises);
        });
    },
    flattenDeep: function(arr) {
        var fn = arguments.callee;
        return arr.reduce(function(a, x) {
            return a.concat(Array.isArray(x) ? fn(x) : x);
        }, []);
    }
}

For an array reflecting the full directory structure, call as follows :

dirOpener.getDirectoryStructure(rootPath)
.then(function(results) {
    console.log(results);
})
.catch(function(e) {
    console.log(e);
});

Or, for a flattened array containing just the filename objects :

dirOpener.getDirectoryStructure(rootPath)
.then(dirOpener.flattenDeep)
.then(function(results) {
    console.log(results);
})
.catch(function(e) {
    console.log(e);
});
Roamer-1888
  • 19,138
  • 5
  • 33
  • 44
  • There is no reason to use `try catch` in the `Promise` constructor callback. All exceptions in there will automatically reject the promise (that's the large advantage over the deferred pattern). – Bergi Oct 29 '15 at 21:16
  • @Bergi, I thought I read somewhere that Bluebird worked that way but native Promise didn't? – Roamer-1888 Oct 29 '15 at 21:42
  • No, all ES6-compatible promise libraries work that way. Which includes the native implementation :-) – Bergi Oct 29 '15 at 21:50