Q promises not working in lodash reduce function

Question

I am trying to return the file in a directory with the earliest modification date. This approach seems to be failing in the createFileDateMap function. I want to reduce over an array of file paths and create an object with the filename and mod-dates. The getModDate function is an asynchronous fs.lstat call. I can't seem to set acc of my reducer to the values from within a .then() block. I'm not sure how to achieve reducing when the values depend on asynchronous call

var _ = require('lodash'),
    fs = require('fs'),
    path = require('path'),
    Q = require('q');

function checkDir(dir) {
    // Check if given path is valid directory
    var deferred = Q.defer();
    fs.lstat(dir, deferred.makeNodeResolver());
    return deferred.promise.invoke('isDirectory');
}

function getFiles(dir) {
    // Get all files within a directory
    var deferred = Q.defer();
    fs.readdir(dir, deferred.makeNodeResolver());
    return deferred.promise;
}

function makeFullPathFileArr(dir, files) {
    // Return array of full paths
    return _.map(files, function(file) {
        return path.join(dir, file);
    });
}

function getModDate(file) {
    // Return modification date of file
    var deferred = Q.defer();
    fs.stat(file, deferred.makeNodeResolver());
    return deferred.promise.get('mtime');
}

function createFileDateMap(filesArr) {
    // Return an obj of file paths and modification dates as Date objects
    // {{ file1: Date, file2: Date }}
    var fileDateMap = _.reduce(filesArr, function(acc, file) {
        getModDate(file)
            .then(function(modDate) {
                acc[file] = moment(modDate);
            });
        return acc;
    }, {});
    return fileDateMap;
}

function getMinDateFile(mapObj) {
    // return the file name which has the earliest modification date
    var dates = _.transform(mapObj, function(result, date, key) {
        result[key] = new Date(date);
    });
    var minDate = new Date(Math.min.apply(null, _.values(dates)));
    var invertedMapObj = _.invert(mapObj);

    return invertedMapObj[minDate];
}

var dir = '../reports';
checkDir(dir)
    .then(function(exist) {
        if(exist) {
            getFiles(dir)
                .then(function(fileNames) {
                    return makeFullPathFileArr(dir, fileNames);
                })
                .then(function(fullpathsArr) {
                    return createFileDateMap(fullpathsArr);
                })
                .then(function(fileAndDatesObj) {
                    console.log(getMinDateFile(fileAndDatesObj));
                });
        }
    })
    .catch(function(err) {
        console.log(err);
    });

You're not returning from the reduce function. That's your bug. Also, properly promisifying your APIs like SLaks suggested can really help with your code. — Benjamin Gruenbaum, May 11 '14 at 20:10
I was looking through the Q docs. How would I use those methods? I thought using makeNodeResolver() was more appropriate here. — Ptrkcon, May 11 '14 at 20:10
@Ptrkcon to be fair, Q is kind of slow, and gives kind of bad stack traces. If you still can I'd definitely switch to Bluebird promises that are much more debuggable and are two orders of magnitude faster. If you intend to still use Q, I'd use `.denodify` that takes a function and promisifies it - also see http://stackoverflow.com/questions/22519784/how-do-i-convert-an-existing-callback-api-to-promises — Benjamin Gruenbaum, May 11 '14 at 20:11
@BenjaminGruenbaum I added return acc; into my reduce function and I still get undefined as a return. — Ptrkcon, May 11 '14 at 20:14
Your return statement is still wrong after your recent edit. You need to return from the reduce, not from the `.then`. Are you sure you're not just looking for `Q.all` that waits for an array of promises to all resolve? `Q.all(filesArr.map(getModDate)).then(function(files){...` — Benjamin Gruenbaum, May 11 '14 at 20:14
@BenjaminGruenbaum I edited a Q.all that seems to work. How is using .denodify or Q.invoke() more appropriate? This solution seems to be working. I will look into Bluebird too! — Ptrkcon, May 11 '14 at 20:27
@Ptrkcon please do not edit the answer into the question, keep the question at the initial revision - otherwise it makes no sense as a question. As for promisifying APIs manually, it's more risky since promises are throw safe (one of the BIG advantages for them) and have sensible exception handling - that requires that functions that return promises also don't throw synchronously (but rather, return a rejected promise) - a lot of times people miss on the more subtle spots when they convert a callback API to promises themselves, especially with deferred objects. — Benjamin Gruenbaum, May 11 '14 at 20:30

score 3 · Accepted Answer · answered May 11 '14 at 20:39

Thanks to @BenjaminGruenbaum for the help. :)

function createFileDateMap(filesArr) {
    // Return an obj of file paths and modification dates as Date objects
    // {{ file1: Date, file2: Date

    return Q.all(_.map(filesArr, getModDate))
        .then(function(modDates) {
           return _.zipObject(filesArr, modDates);
        });
}

Bergi · Answer 2 · 2014-05-11T20:46:36.043

The problem is that reduce is synchronous and does not know about the promises you use in its callback. You're returning the fileDateMap, which is an empty object. There are two solutions to this problem:

Invoke all the getModDates in parallel, use Q.all to get a promise for the array of dates, and reduce over that in a then callback.

function createFileDateMap(filesArr) {
    // Return an obj of file paths and modification dates as Date objects
    // {{ file1: Date, file2: Date }}
    return Q.all(_.map(filesArr, function(file) {
        return getModDate(file).then(function(date) {
            return {name:file, date:date};
        });
    })).then(function(fileAndDateArr) {
        return _.reduce(fileAndDateArr, function(acc, o) {
            acc[o.file] = moment(o.date);
            return acc;
        }, {});
    });
}

Make the accumlutor a promise for the map object, and reduce "asynchronously" over the array by chaining lots of then calls.

function createFileDateMap(filesArr) {
    // Return an obj of file paths and modification dates as Date objects
    // {{ file1: Date, file2: Date }}
    return _.reduce(filesArr, function(acc, file) {
        return acc.then(function(fileDateMap) {
            return getModDate(file).then(function(modDate) {
                fileDateMap[file] = moment(modDate);
                return fileDateMap;
            });
        });
    }, Q({}));
}

How do you feel about _.zipObject which is essentially doing what your top function is doing? Any disadvantages? — Ptrkcon, May 11 '14 at 21:17
It would be easier to read and probably more efficient :-) My function is just a bit ~~cleaner~~ more pure in the regard that it doesn't matter whether `filesArr` and `modDates` are in the same order, because it stores filename + date in an object. Understand my solution, then use your snippet :-) — Bergi, May 11 '14 at 21:24

Q promises not working in lodash reduce function

2 Answers2