4

I'm new to angular's $q and I'm trying to build a service that makes an API call and returns it back to the controller.

Problem: No matter how I seem to format it, the service returns right before it gets to $http.get(...)

Service:

   // methods: query new, get existing
  makeRequest: function(url, following) {
      // create promise
      var deferred = $q.defer();

      $http.get(url, {
          params: {
              "following": JSON.stringify(following)
          }
      })
          .then(function(res) {

              console.log(res);
              deferred.resolve(res.data);
          });

      return deferred.promise;
  },

  getFeed: function(user) {
      console.log('>> userService[getUser]: retrieving user...');


      if (!this.activities) {
          // Request has not been made, setting user profile. 
          console.log('>> userService[getUser]: No user stored, making request...');
          var following = this.compileArray(user);
          console.log(following);
          this.activities = this.makeRequest('api/network/activities', following);
      };

      // Return the myObject stored on the service
      return this.activities;
  }

Controller

$scope.recentActivity = activityService.getFeed(profile);

// also tried
activityService.getFeed(profile).then(function (res) {
                  $scope.recentActivity = res;
                  console.log(res);
                }); 

EDIT : 9:40am 05/06/2015

If possible, I'd like to retrieve the activity list in the controller from the service in the same way I would if it were new (using .then). Is that possible/ bad practice?

  getFeed: function(user) {
    if (!this.activities) { 
      ...
    } else {
      feedPromise = $q(function(resolve){ resolve(this.activities) });
      console.log(feedPromise);
      // returns: {$$state: Object, then: function, catch: function, finally: function}

      feedPromise.then(function(res) {
        console.log(res);
        // returns: undefined
      });

      console.log(that.activities);
      // Works, returns list of activities. 

   }
dmr07
  • 1,388
  • 2
  • 18
  • 37
  • any error in your console.. also whether `makeRequest` is getting called – Arun P Johny May 05 '15 at 03:02
  • Yes, it does make it to `makeRequest ` in the console, and it's not logging any errors. The very next log I see is the one in the controller, after the service is called. It's not waiting for the $http to finish. – dmr07 May 05 '15 at 03:08
  • 2
    `$http` is already a promise, there is really no reason to wrap it in another promise. and because it is a promise, your code will not wait for the `$http` call to complete. – Claies May 05 '15 at 03:09
  • whether `console.log(res);` is getting logged in your console... – Arun P Johny May 05 '15 at 03:13
  • @Arun, yes it is getting logged, but it occurs way after the controller finishes running, and it doesn't set the variable in the controller. @Claises, I've read about it, but when I remove $q and its constituents (defer, resolve, etc.), it `makeRequest` still return before the API sends back a response. – dmr07 May 05 '15 at 03:16
  • Have you tried to pull data from service in resolve block ?. Controller is not get loaded until resolve block is not resolved – Bharat Bhushan May 05 '15 at 03:18
  • that is how ajax works.. as it is executed asynchronously... since that is executed what is the problem... your `$scope.recentActivity = res;` also should get executed – Arun P Johny May 05 '15 at 03:19
  • 1
    yes, it *will* return back before the API sends the response, that's how promises work. the `.then` callback is executed when the data is returned, but the rest of the code continues to process until this happens. – Claies May 05 '15 at 03:24
  • I don't see any error handling, maybe the request is failing. – Miguel Mota May 05 '15 at 03:31

3 Answers3

5

There is no need to use a $q.defer unless you are converting a non-promise based API with callbacks into a promise-based API (and even then, it is recommended to use $q(function(resolve, reject){...})).

$http already returns a promise - just return that (or a chained .then promise);

var httpResponsePromise = $http.get(url); // returns a promise
var actualDataPromise = httpResponsePromise.then(function(resp){ return resp.data; });

return actualDataPromise;

or shorter (and typical):

return $http.get(url).then(function(response){ 
   return response.data;
});

Second, a promise-returning API returns the promise - not the result - right away, synchronously. You need a .then to get the result.

Lastly, once an API is async, it should always be async - don't convert it to a sync or a sometimes-sync API. So, everywhere, all the way to the end recipient of the data, you need to use a .then handler.

So, your service API can be made quite simple:

makeRequest: function(url, following){
   return $http.get(url, {params: { "following": following }})
             .then(function(response){
                return response.data;
             });
},

getFeed: function(user) {
   var that = this;

   var feedPromise;

   if (!that.activities) {
      var following = this.compileArray(user);

      feedPromise = this.makeRequest('api/network/activities', following)
          .then(function(activities){
             that.activities = activities;
             return activities;
          });
   } else {
      feedPromise = $q(function(resolve){ resolve(that.activities); });
      // or you could have cached the old feedPromise and returned that
   }

   return feedPromise;
}

The usage in the controller is just like with any other promise-based API:

activityService.getFeed(profile)
   .then(function(activities) {
      $scope.recentActivity = activities;
   }); 
New Dev
  • 48,427
  • 12
  • 87
  • 129
  • thanks for the answer, I tried it and it's working. I still don't fully grasp why I need a `return` on the $http.get? (I've noticed there are several posts on stackoverflow that don't use it ~ for example: http://stackoverflow.com/questions/18101875/how-do-i-return-data-from-a-http-get-inside-a-factory-in-angularjs) – dmr07 May 05 '15 at 15:51
  • `return http.get().then()` returns the promise generated by `.then`. So, it returns a promise. There is no need to create a new promise via `$q.defer`. This is a commonly used [anti-pattern](https://github.com/petkaantonov/bluebird/wiki/Promise-anti-patterns#the-deferred-anti-pattern). To really understand why, read about promise chaining, and a bit about [why promises exist](https://gist.github.com/domenic/3889970) – New Dev May 05 '15 at 16:49
  • Hi New Dev, I haven't gotten a chance to take a deeper look into this, got a big project that we're wrapping up. I'll let you know by this weekend. – dmr07 May 05 '15 at 23:37
  • Having trouble... I put a `feedPromise.then(function(res) { console.log(res) })` inside the `else`, and it's returning undefined. How can I access the data once it's stored? – dmr07 May 06 '15 at 15:33
  • @danm07, what is returning `undefined`? You mean that `console.log(res)` outputs `undefined`? – New Dev May 06 '15 at 15:56
  • Output is `undefined`. i.e. if activities is already stored in the service, the controller `$scope.recentActivity` is `undefined`. – dmr07 May 06 '15 at 16:10
  • And you are still returning `feedPromise` or are you returning the promise generated by `feedPromise.then()`? If it's the latter, then the reason you are getting `undefined` is because you are not returning the data within `.then` (you just do `console.log`) – New Dev May 06 '15 at 16:24
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/77108/discussion-between-danm07-and-new-dev). – dmr07 May 06 '15 at 16:51
  • 1
    @danm07, yeah, there was a bug in `resolve(this.activities)` - `this` was not referring to the service instance - I fixed it in the answer by using `that` – New Dev May 06 '15 at 17:20
  • I should have caught that... been staring at this thing for too long 0.0'. Anyway thanks New Dev, people like you really make stackoverflow a nice place for new developers. – dmr07 May 06 '15 at 17:25
0

After learning about the deferred anti-pattern as pointed out in the comments by @New Dev, I have edited this answer. Please see @New Dev's answer for a detailed explanation. Thanks @New Dev.

Service

    makeRequest: function(url, following) {
        return $http.get(url, {
                params: {
                    "following": JSON.stringify(following)
                }
            }).then(function(res) {
                return res.data;
            });
    },    
    getFeed: function(user) {

        var that = this;

        if (!that.activities) {
            var following = that.compileArray(user);

            return that.makeRequest('api/network/activities', following)        
                .then(function(activities){
                    that.activities = activities;
                    return that.activities;
                });
        } else {
            return $q(function(resolve) {
                resolve(that.activities);
            });
        }    
    }

Controller

    activityService
        .getFeed(profile)
        .then(function (activities) {
            $scope.recentActivity = activities;                 
        });
  • Yes, give me a few minutes – New Dev May 05 '15 at 04:16
  • 1
    First, you are using a [deferred anti-pattern](https://github.com/petkaantonov/bluebird/wiki/Promise-anti-patterns#the-deferred-anti-pattern) in your solution. Second, your `getFeed` will never get resolved if `this.activities` has already been set. And you had an error with `.success` (but you fixed it now). And lastly, your answer is mostly code with no explanation – New Dev May 05 '15 at 04:26
  • Thanks for pointing that out. I assumed that he wanted to do some post processing on the data in the makeRequest method before resolving it to the getFeed method. – Rathish Cholarajan May 05 '15 at 04:33
  • Are you talking about the "never get resolved" point? Then it has nothing to do with postprocessing - in your solution the entire `if (!that.activities)` gets skipped and the caller with left with a promise that will never get resolved. – New Dev May 05 '15 at 04:37
  • I just read about the deferred anti pattern. I understand now. Thanks a lot. – Rathish Cholarajan May 05 '15 at 04:52
-1

i think you need to use .success instead of .then if you want to have the response object.

$http.get(url, {
         params: {
          "following": JSON.stringify(following)
      }
  })
      .success(function(res) {

          console.log(res);
          deferred.resolve(res.data);
      });
kwangsa
  • 1,701
  • 12
  • 16
  • That is actually the opposite - `.then` gets the `response` object and you can get the data with `response.data`. `.success` gets the `data` as a first parameter – New Dev May 05 '15 at 03:51
  • I tried replacing it with `.success`, no luck. Still the same problem – dmr07 May 05 '15 at 03:51