4

According to AngularJS, my $http call through a service from my controller is returning undefined?

What seems to be the issue here? I am trying to return the data called, but once passed to the controller the data becomes undefined?

JavaScript

var myStore = angular.module('myStore', [])

    .controller('StoreController', ['$scope', 'dataService', function ($scope,  dataService) {
      $scope.products = dataService.getData();
    }])

    .service('dataService', ['$http', function($http) {
      this.getData = function() {
        $http.get('assets/scripts/data/products.json')
              .then(function(data) {
                return data;
              });
      };
    }]);

HTML

<div class="content">
  <ul>
    <li ng-repeat="product in products.products">{{product.productName}}</li>
  </ul>
</div>

I understand that $http, $q, and $resource all return promises, but I thought I had covered that with .then.

Arslan Ali
  • 17,418
  • 8
  • 58
  • 76
Aaron Brewer
  • 3,567
  • 18
  • 48
  • 78
  • 2
    `getData()` doesn't have a `return` statement of its own. The `return data` instead applies only to the callback, `function(data)`. Though, `$http` is also asynchronous, meaning `getData()` doesn't wait for `data` to become available -- [How to return the response from an asynchronous call?](http://stackoverflow.com/questions/14220321/how-to-return-the-response-from-an-asynchronous-call) Note especially the mentions of Promises (`.then()`-ables). – Jonathan Lonowski Apr 20 '15 at 03:09
  • Just add `return this` as the last line of the service, hope it will solve your problem – Ali Adravi Oct 19 '15 at 20:35

2 Answers2

5

The problem could be that you are not returning the promise created by $http.get in your dataService.getData function. In other words, you may solve your undefined issue by changing what you have to this:

.service('dataService', ['$http', function($http) {
    this.getData = function() { 
        return $http.get...
    };
}

If you had multiple calls to $http.get within dataService.getData, here is how you might handle them.

.service('dataService', ['$http', function($http) {
    this.getData = function() {
        var combinedData, promise;
        combinedData = {};
        promise = $http.get(<resource1>);
        promise.then(function (data1) {
            combinedData['resource1Response'] = data1;
            return $http.get(<resource2>);
        });
        return promise.then(function (data2) {
            combinedData['resource2Response'] = data2;
            return combinedData;
        });
    };
}]);

A much cleaner way, however, would be to use $q.all

.service('dataService', ['$http', '$q', function($http, $q) {
    this.getData = function() {
        var combinedData, promises;
        combinedData = {};
        promises = $q.all([
            $http.get(<resource1>),
            $http.get(<resource2>)
        ]);
        return promises.then(function (allData) {
            console.log('resource1 response', allData[0]);
            console.log('resource2 response', allData[1]);
            return allData;
        });
    };
}]);
maxenglander
  • 3,991
  • 4
  • 30
  • 40
  • What if I had multiple functions in this service that made $http calls to different resources? Thus the problem therein lies having the return that high in the hierarchy. – Aaron Brewer Apr 20 '15 at 03:12
  • I guess the answer to that question would depend on how you would want `getData` to behave if in case of partial failure (one call failed, another succeeded). However, I'll update my answer with an example of handling multiple calls. – maxenglander Apr 20 '15 at 03:15
  • @iamaaron You can use [`$q.all(promises)`](https://docs.angularjs.org/api/ng/service/$q#all) to collect the results from multiple `$http` requests, along with any other promises. – Jonathan Lonowski Apr 20 '15 at 03:20
  • 1
    You have a mistake - it shouldn't be `return this.getData` - you only need to return a promise within `getData` function - not the `this.getData` itself – New Dev Apr 20 '15 at 04:25
  • You're right @NewDev, embarrasing mistake on my part. – maxenglander Apr 20 '15 at 05:33
3

You're problem does lie in the fact that you are not returning a promise but as you stated in @maxenglander's post you may have multiple http calls involved which means you should start creating and resolving your own promise using $q:

.service('dataService', ['$http', '$q', function($http, $q) {
      return $http.get('assets/scripts/data/products.json')
          .then(function(data) {
            //possibly do work on data
             return <<mutated data>>;
            });
}];

or if you have multiple http calls and need to do some combination work you can do something $q.all:

.service('dataService', ['$http', '$q', function($http, $q) {
      var p1 = $http.get('assets/scripts/data/products.json');
      var p2 = $http.get('assets/scripts/data/products2.json');
      return $q.all([p1, p2]).then(function(result){
         //do some logic with p1 and p2's result
         return <<p1&p2s result>>;
       });
}];

then in your controller you will need to do:

.controller('StoreController', ['$scope', 'dataService', function ($scope,  dataService) {
     dataService.getData().then(function(result){
         $scope.products = result;
     });
}]);

What this allows in your service is now you can do complex calls like say call two webservices inside and wait till they are both complete then resolve the promise. What I'm trying to express here is that you don't need to return the promise provided by the $http.get function, but since you are doing an async action you DO need to return some promise that will be later fulfilled and acted on.

dbarnes
  • 1,803
  • 3
  • 17
  • 31
  • 2
    You are promoting an anti-pattern - a [deferred anti-pattern](https://github.com/petkaantonov/bluebird/wiki/Promise-anti-patterns#the-deferred-anti-pattern) - with this approach – New Dev Apr 20 '15 at 04:22
  • I just disagree with that post, as I think it promotes more readability than "not understanding promises" – dbarnes Apr 20 '15 at 16:20
  • Promises are more than glorified callbacks - they promote code composition that parallels that of synchronous code, with respect to order of execution and exception handling. So, supposed improved readability is not a justification for under-using promising and unnecessarily using deferred – New Dev Apr 20 '15 at 16:24
  • Even though I do not totally agree with it as an anti pattern, I eventually can see something where you get into "return hell". I do see the point how an error could get swallowed, which has made me change the code. But my opinion will not change promises anytime soon. – dbarnes Apr 20 '15 at 16:37