2

I've created a simple example of this with the below code. The goal is to be able to used the deferred promise from Angular's $q service but have $q within a callback that itself returns a result that is to be handled in the main controller.

I recognize the error is clearly that the $q's promise needs to be returned immediately so that it can "await" the result and that placing that promise within a callback inhibits it from being returned immediately. Thus, the code below is clearly an incorrect strategy.

My question is to ask what the best practice would be to achieve a comparable utility to the above described desire, including the presence of both a callback and need to return a promise.

function asyncGreet(name, cb) {
  cb(name)
}

function okToGreet(name) {
  return name.length > 10
}

var promise = asyncGreet('Robin Hood', function(name) {
    var deferred = $q.defer();

  setTimeout(function() {
    deferred.notify('About to greet ' + name + '.');

    if (okToGreet(name)) {
      deferred.resolve('Hello, ' + name + '!');
    } else {
      deferred.reject('Greeting ' + name + ' is not allowed.');
    }
  }, 1000);

  return deferred.promise;
});

promise.then(function(greeting) {
  console.log('Success: ' + greeting);
}, function(reason) {
  console.log('Failed: ' + reason);
});
kuanb
  • 1,618
  • 2
  • 20
  • 42
  • Is `asyncGreet` calling its callback synchronously on purpose? – Bergi Jul 06 '15 at 23:03
  • 2
    Not answering your question but replace the syntax `if (name.length > 10) { return true } else { return false }` with the syntax : `return name.length > 10;`. The latter is more sexy:) – Michel Jul 06 '15 at 23:04
  • Why doesn't `asyncGreet` return a promise? What exactly is your "*need*" to use a callback? – Bergi Jul 06 '15 at 23:04
  • 1
    @Bergi the use case is using `gapi`, Google's OAuth client side library (https://developers.google.com/api-client-library/javascript/reference/referencedocs). I need to perform some function related to that library, which involves callbacks, and then send aspects of the result(s) to the `$scope` in the main controller. – kuanb Jul 06 '15 at 23:08
  • @Bergi I should also answer your first comment - yes, it is calling `asyncGreet` synchronously - the purpose was just to create a quick example the demonstrated the situation I had found myself in. – kuanb Jul 06 '15 at 23:10
  • @kuanb: Which of the `gapi` methods are you using? It would help a lot to include your actual code, or at least mark those parts where you're calling into foreign code. – Bergi Jul 06 '15 at 23:10
  • @Bergi: https://github.com/urbanlaunchpad/ftdb/blob/master/static/scripts/user/services/googleAuth.js See LOC 22, 37. Both functions call `handleAuthResult`. If `authResult.status.signed_in` is true, I need to update a `$scope` variable in my main controller. – kuanb Jul 06 '15 at 23:13
  • 1
    To me, it looks like all of `checkAuth`, `handleClientLoad` and `handleAuthClick` should return promises. You should [promisify](http://stackoverflow.com/q/22519784/1048572) the `gapi` methods, and then never use deferreds anywhere else. – Bergi Jul 06 '15 at 23:20
  • @Bergi, thanks for the suggestion. To clarify, are you referring specifically to the use of ES6 Promises in this case? – kuanb Jul 06 '15 at 23:28
  • 1
    @kuanb: No, you can use whatever promise library fits you (hey, they're interoperable!). You probably want to use `$q` promises in your case. – Bergi Jul 06 '15 at 23:30
  • See http://stackoverflow.com/questions/22519784/how-do-i-convert-an-existing-callback-api-to-promises – Benjamin Gruenbaum Jul 07 '15 at 09:47

2 Answers2

2

Well I actually figured it out. You create deferred and then pass it into the callback. Should have been obvious to me before I posted this but perhaps it'll help someone else who got confused as I did:

function asyncGreet(name, cb) {
  var deferred = $q.defer();

  setTimeout(function() {
    var foo = null;
    cb(name, deferred)
  }, 1000);

  return deferred.promise;
}

var promise = asyncGreet('Robin Hood', function(name, deferred) { 
  if (name.length > 10) {
    foo = 'Hello, ' + name + '!';
  } else {
    foo = 'Greeting ' + name + ' is not allowed.';
  }
  deferred.resolve(foo);
});

promise.then(function(greeting) {
  console.log('Success: ' + greeting);
}, function(reason) {
  console.log('Failed: ' + reason);
});
kuanb
  • 1,618
  • 2
  • 20
  • 42
  • 2
    This seems a bit backwards with regards to where the `cb` function is called. Callbacks are not typically called synchronously (as you did in `asyncGreet`. In your example, which function models the `gapi` API (and thus, not under your control)? Is it `asyncGreet`? – New Dev Jul 07 '15 at 03:45
  • @NewDev the purpose was to determine a solution within the parameters of the situation outlined in the question's example code - which had callbacks. See Bergi's comments beneath that wrt further discussion of improving the cb structure itself. – kuanb Jul 07 '15 at 13:44
  • You provided an "answer" on SO (whether you're the OP or not), which could be useful to some future visitor. So my comment was for you to address this issue in your answer, for the confusion that I referred to above. I don't think it's enough to just say - see comments elsewhere – New Dev Jul 07 '15 at 16:56
  • @NewDev thank you for the additional comments, I had misread your first comment and made some changes to the example code to improve it's readability and remove the confusing aspect that was prior there. I will say that your tone can come across a little condescending, particularly the use of quotes around "answer." I'm assuming that you, too, see the value of SO in being a resource for all to learn - that includes those trying to craft the best answer. Such language and tone as you employ serves no other purpose than to put down the author, which is frustrating. – kuanb Jul 07 '15 at 18:56
  • 1
    My apologies for the tone - it wasn't my intent to be condescending. The quotes around the word "answer" was not to diminish it, but rather emphasize my point that it is "an answer on SO" - that is, it should be complete and self-contained. This was rather my comment to your reply of "see comments elsewhere" - not to the answer itself. The first comment *was* about the answer, which I hoped to nudge you to improve - but you sort of ignored it. – New Dev Jul 07 '15 at 19:12
  • Understood, thanks for the follow up. No harm done, I think ultimately it was a communications failure - my misunderstanding of your initial comment that sparked this. Thanks again. – kuanb Jul 07 '15 at 19:18
  • 1
    Btw, this approach would only work if the API itself (i.e. `asyncGreet`) passed the `deferred` object to the callback. So, that is ultimately what was confusing. Non-promise/callback-based APIs typically don't have the notion of a deferred, and hardly ever pass that as a parameter to the callback function. So, again... is `asyncGreet` under your control or is it a 3rd party library? – New Dev Jul 07 '15 at 19:26
  • In my case `asyncGreet` (I am using `gapi`, Google JS OAuth API library) is not under my control. – kuanb Jul 09 '15 at 15:05
  • Right... so the solution likely cannot be `cb(name, deferred)`, because it is not in your control what parameters are passed to the callback function. – New Dev Jul 09 '15 at 16:50
  • Hrm right. I see what you are saying - so the situation is in fact that `gapi` uses promises and the structure I was mimicking used a callback that was executed once the last `.then()` was successful. So, as Bergi had suggested in the main question's comments, it would be best to promisify that component rather than use a poor mix of callbacks and promises as is currently occurring. – kuanb Jul 09 '15 at 17:03
2

To consolidate the comments and to address the other answer's somewhat confusing description of an async API, I decided to provide an answer:

If we suppose that there is some async non-promise / callback-based API, for example, asyncGreet, it could be mocked like so:

function asyncGreet(name, cb){
  // simulate async
  setTimeout(function(){
    someCondition ? cb({message: "Hello, " + name + "!"}) :
                    cb({error: "Greeting " + name + " is not allowed."});
  }, 2000);
}

(for the purposes of this example, asyncGreet is a 3rd party API not in our control)

To convert this to a $q promise-based API, you would use a $q (either with $q.defer or with $q constructor - in fact, I just noticed that Angular's documentation shows the $q-constructor approach).

So, for example, you could create a greeterSvc service:

app.factory("greeterSvc", function greeterSvcFactory($q){
  return {
    greet: function(name){

      return $q(function(resolve, reject){            
        asyncGreet(name, function cb(data){
          if ('error' in data) {
            reject(data.error);    // extract reason
          } else {
            resolve(data.message); // extract greeting message
          }       
        });

      });
    }
  }
})

The consumer of the greeterSvc.greet API could then .then it and, for example, log something - just like you did (although I would use .catch instead)

greeterSvc.greet("Robin Hood")
          .then(function(greeting) {
             console.log('Success: ' + greeting);
          })
          .catch(function(reason) {
             console.log('Failed: ' + reason);
          });
New Dev
  • 48,427
  • 12
  • 87
  • 129