-1

I use some Promise code used in my application as below;

import { Promise, resolve, all } from 'rsvp';


someAction: function(secId, fld, callback) {
    var self = this;
    var section = self.findSection(self.get('allSecs'), secId);
    var myPendingPromise = section.myPendingPromise || resolve();
    myPendingPromise = myPendingPromise.then(function(){
        return self.myCustomPromise(secId, fld, callback);
    });
    set(section, 'myPendingPromise', myPendingPromise);
},


myCustomPromise: function(secId, fld, callback){
    var self = this;
    return new Promise(function(resolve, reject){
        var deferred = self.myCustomRule(someFlds); //Makes an API call
        deferred.then(function(response) {
            resolve(response);
        }, function(){
            resolve(true);
        });
    });
},

Now, I am a bit confused why the following lines are added specifically;

var myPendingPromise = section.myPendingPromise || resolve();
myPendingPromise = myPendingPromise.then(function(){
    return self.myCustomPromise(secId, fld, callback);
});
set(section, 'myPendingPromise', myPendingPromise);

Also, I did not find "myPendingPromise" used anywhere else apart from this function. Is there some pattern which I need to be aware of to be able to understand this code? It would be great to understand just the usage of these 3 lines of code above.

copenndthagen
  • 49,230
  • 102
  • 290
  • 442

1 Answers1

2

What is that

It looks like an attempt to solve concurrency problem by adding all new promises to promise chain (queue). I prepared a simplified example based on your code that demonstrates how it works.

What exactly each line does:

//Extract pending promise from section object. If undefined, use resolve() 
//to create and resolve dummy promise:
var myPendingPromise = section.myPendingPromise || resolve();

//Add new promise to chain, so it would start after 
//pending promise is resolved:
myPendingPromise = myPendingPromise.then(function(){
    return self.myCustomPromise(secId, fld, callback);
});

//Save new promise chain into section object:
set(section, 'myPendingPromise', myPendingPromise);

Why it's a bad solution to concurrency problem (my opinion)

  1. A bit hard to read and understand
  2. If promise takes a long time to finish and someAction is called many times, queue can grow uncontrollably long
  3. It seems that nothing indicates to user that something is running

What is a good solution (again, my opinion)

  1. Use library like ember-concurrency to manage concurrency
  2. Avoid using queue strategy for concurrency problems. If you need to use "queue" strategy, take measures to limit queue length
  3. Add some indication so user sees that button worked and request is happening. It's easy to do using ember-concurrency
Gennady Dogaev
  • 5,902
  • 1
  • 15
  • 23
  • Thanks a lot for the detailed solution with comments/example....So if I have to summarize my understanding, it seems like the code is trying to prevent the execution for multiple actions that might take place (say from the UI) simultaneously and is instead queuing them up. Would that be correct ? – copenndthagen Jan 04 '20 at 12:18
  • Just one quick followup question; Is it possible to have the same behavior using async/await i.e. Make the code explicitly wait for the earlier promise to be resolved ? – copenndthagen Jan 05 '20 at 08:39
  • @testndtv I don't think it can be effectively re-written to async/await syntax – Gennady Dogaev Jan 05 '20 at 12:23