Proper way to recurse Q.Promise

Question

I do something like this. It seems to work for me, but is it OK? Is there any better way?

function myPromise(options) {
  return Q.Promise(function(resolve, reject, notify) {
    doSomthingAsync(options, function(resp) {
       notify(resp);
      if (resp.nextPageToken) {
        options.pageToken = resp.nextPageToken;
        myPromise(options).then(resolve, reject, notify);
      } else {
        resolve(resp);
      }
    });
  });
}

NOTE: I know mutable options are unwise.

FYI: I'm not sure, but in ReactiveCocoa, there is a same kind of functionality. -[RACSignal subscribe:] https://github.com/ReactiveCocoa/ReactiveCocoa/blob/1e97af8f5681b3685770eb30faf090e64c293290/ReactiveCocoaFramework/ReactiveCocoa/RACSignal.h#L115-L131

You should avoid the notification API and `notify` as it's being replaced in Q 2.0 and will no longer work. It will also never be included in native promises and is overall a poor API. — Benjamin Gruenbaum, Sep 09 '14 at 21:36
If you want an observable or a full blown pub/sub you should consider an event emitter and not a promise - a promise abstracts a one-time operation. — Benjamin Gruenbaum, Sep 09 '14 at 21:38

score 1 · Accepted Answer · edited May 23 '17 at 12:05

1

Is there any better way?

Yes. .then(resolve, reject, notify); is the deferred antipattern.

Better chain the possibly-recursive call with an explicit then:

function myPromise(options) {
  return Q.Promise(function(resolve, reject, notify) {
    doSomthingAsync(options, function(resp) {
      // if (err) return reject(err);
      notify(resp);
      resolve(resp);
    });
  }).then(function(resp) {
    if (resp.nextPageToken) {
      options.pageToken = resp.nextPageToken;
      return myPromise(options);
    } else {
      return resp;
    }
  });
}

And instead of the new Q.Promise call you likely might be able to use one of the callback-function helper methods.

edited May 23 '17 at 12:05

Community

1
1

answered Sep 09 '14 at 15:24

Bergi

630,263
148
957
1,375

The `new Q.Promise` isn't really the deferred anti pattern here as it's required by the (broken) notification API. It gets much simpler if we drop notifications :) – Benjamin Gruenbaum Sep 09 '14 at 21:38
@BenjaminGruenbaum I see that `Q.Promise` is required here for the notification call, but I still think that passing the resolve/reject handlers to a "child" promise is the deferred antipattern. For "broken API", I thought about a helper function `return withImmediateNofication(resp, myPromise(options))` for jobs like this. – Bergi Sep 09 '14 at 21:45
After all, instead of abusing notifications, some kind of FRP Stream API is probably a better fit for this usecase. – Bergi Sep 09 '14 at 21:46
A stream would probably be much much better for this use case. Yes. – Benjamin Gruenbaum Sep 09 '14 at 21:47

Proper way to recurse Q.Promise

1 Answers1