How to avoid passing promise through multiple callbacks to resolve promise? - Deft.js

Question

As my code shows, I currently am passing a promise down two asynchronous callbacks functions (success and callback). I think it should be possible to do this differently, but I have found no other way yet.

Here is my function that creates the promises and waits for them all to be done:

_onModelLoaded: function(model) {
    var promises = [];

    _.each(this._customRecords, function(defect, index) {
        promises.push(this._getHistory(model, defect));
    }, this);

    Deft.Promise.all(promises).then({
        success: function(something) {
            console.log('woot',something);
        }
    });
},

The _getHistory function creates a promise, then passes it into two functions (_onHistoryLoaded and _onRevisionsLoaded) so that once these callbacks are run, the promise gets resolved:

_getHistory: function(model,defect) {
    var deferred = Ext.create('Deft.promise.Deferred');

    model.load(Rally.util.Ref.getOidFromRef(defect.RevisionHistory), {
        scope: this,
        success: function(record) {
            this._onHistoryLoaded(defect, record, deferred);
        }
    });

    return deferred.promise;
},

_onHistoryLoaded: function(defect, record, def) {
    record.getCollection('Revisions').load({
        fetch: ['RevisionNumber', 'CreationDate', 'User', 'Description'],
        scope: this,
        callback: function(revisions) {
            this._onRevisionsLoaded(revisions, defect, def);
        }
    });
},

_onRevisionsLoaded: function(revisions, defect, def) {

    _.each(revisions, function(revision, revisionIndex) {
        // Do stuff with each revision
    }, this);

    def.resolve(defect);
},

I do not necessarily need the final defect to be passed through the promise, I just put it in the resolve statement for testing.

Note: My code does run properly, I am just looking for simplification.

Is there a way to avoid creating a promise only to pass it through a few async functions to get it resolved?

score 2 · Answer 1 · answered Dec 25 '13 at 00:07

I'm going to try and offer some useful tips, but first a few caveats.

I don't know anything about the domain problem you're trying to solve which makes it difficult to decide on the best names for things or exactly where each unit of functionality fits into the grand scheme of things.
The library you're using doesn't appear to support the Promise API as used in Promises/A+ and the next version of JavaScript. You can read the spec for true promises at: http://promisesaplus.com/ and I maintain a short list of my favourite implementations at http://www.promisejs.org/implementations/

I'll try writing your code as I would write it with "Promise" which is the first of those implementations and the one I wrote myself.

Since onHistoryLoaded is actually an asynchronous operation, change it's name to the action it performs and have it return a promise. Do the same with onRevisionsLoaded and make getHistory do just one job:

_onModelLoaded: function(model) {
    var promises = _.map(this._customRecords, function(defect, index) {
        this._getHistoryRevisions(model, defect).then(function (revisions) {
            _.each(revisions, function (revision, revisionIndex) {
              // Do stuff wich each revision
            }, this);
        }.bind(this));
    }, this);

    Promise.all(promises).done(function(something) {
        console.log('woot',something);
    });
},

_getHistoryRevisions: function (model, defect) {
    return this._getHistory(model, defect).then(function (record) {
        return this._getRevisions(record);
    }.bind(this));
},

_getHistory: function(model, defect) {
    return new Promise(function (resolve, reject) {
        model.load(Rally.util.Ref.getOidFromRef(defect.RevisionHistory), {
            scope: this,
            success: function(record) {
                resolve(record);
            }
        });
    }.bind(this));
},

_getRevisions: function(record) {
    return new Promise(function (resolve, reject) {
        record.getCollection('Revisions').load({
            fetch: ['RevisionNumber', 'CreationDate', 'User', 'Description'],
            scope: this,
            callback: function(revisions) {
                resolve(revisions);
            }
        });
    }.bind(this));
}

Wherever possible, it's best to name the functions after the thing they do, not what causes them. They shouldn't really need to be coupled to what happens before them or after them. A large part of the idea behind promises is that it makes it easier to get back to the synchronous model of programming where I call a function, and it does a thing, then returns a value. The less awareness a function has of where it will be used, the easier it is to re-use somewhere else :)

score 0 · Accepted Answer · answered Apr 09 '14 at 18:23

The answer provided by @ForbesLindesay was very helpful. However, I was using Deft 0.8.0 so there were some differences in the implementation of these promises. Here is how I got the chain to work:

_onModelLoaded: function(model) {
    var promises = [];

    _.each(this._customRecords, function(defect, index) {
        promises.push(this._getHistoryRevisions(model, defect).then(function (revisions) {
            // do stuff with revisions
        }.bind(this)));
    }, this);

    Deft.Promise.all(promises).then(function(something) {
        console.log('woot',something);
    }.bind(this));
},

_getHistoryRevisions: function (model, defect) {
    return this._getHistory(model, defect).then(function (record) {
        return this._getRevisions(record);
    }.bind(this));
},

_getHistory: function (model, defect) {
    var deferred = Ext.create('Deft.promise.Deferred');

    model.load(Rally.util.Ref.getOidFromRef(defect.RevisionHistory), {
        scope: this,
        success: function(record) {
            deferred.resolve(record);
        }
    });

    return deferred;
},

// grabbing the revisions for each defect
_getRevisions: function(record) {
    var deferred = Ext.create('Deft.promise.Deferred');

    record.getCollection('Revisions').load({
        fetch: ['RevisionNumber', 'CreationDate', 'User', 'Description'],
        scope: this,
        callback: function(revisions) {
            deferred.resolve(revisions);
        }
    });

    return deferred;
},

The main difference comes from the last two functions, _getHistory and _getRevisions - instead of returning a new Promise(...), I had to create a deferred object with Ext.create('Deft.promise.Deferred'); to return and resolve within the callbacks.

Hopefully this helps anyone else with this issue of using an older version of DeftJS

How to avoid passing promise through multiple callbacks to resolve promise? - Deft.js

2 Answers2