1

I frequently see tutorials and snippets online about Angular services to make $http calls that return both the $http promise and some data. If the promise is returned to the controller, what is the point of returning the data in the service? I don't even understand to where it is returned. Here's an example of what I mean:

 // Function of MyStuffService:
 function getStuff() {
    return $http.get('/api/stuff')
        .success(function(data) {
            // Why return data here? How could I even get this returned value?
            return data;
        })
        .error(function(data) {
            console.error(data);
        });
}

// Controller:
function getStuff() {
    MyStuffService.getStuff()
        .success(function(data) {
            $scope.stuff = data;
        })
}

Can't I just rewrite my service function as:

 // Function of MyStuffService:
 function getStuff() {
    return $http.get('/api/stuff')
        .error(function(data) {
            console.error(data);
        });
}

And let the controller get the data from the returned promise? I feel like I'm not understanding something here. Any help is greatly appreciated.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
Kevin Salvesen
  • 293
  • 1
  • 13
  • 1
    What tutorials, and what snippets are you referring to?.. returning the data in the success callback is definitely strange. – Faris Zacina Feb 26 '15 at 17:10
  • For example [here](http://stackoverflow.com/questions/17646034/what-is-the-best-practice-for-making-an-ajax-call-in-angular-js/17646781#17646781). There's a comment in there about stuff that only works in older versions of Angular, but I figured the first part was OK. So I should avoid that pattern? It would be fine to only do `function getStuff() {return $http.get('/api/stuff');}` in my service? – Kevin Salvesen Feb 26 '15 at 17:15
  • Yes.. i think its perfectly fine. – Faris Zacina Feb 26 '15 at 17:16
  • @Klodo: That answer doesn't use `success`, it uses `then`, from which you can indeed `return`. Admittedly, the identity function doesn't make much sense as a callback though. – Bergi Feb 26 '15 at 17:16
  • @Bergi: I see, thank you. I guess should go read about the differences between success/error and then. I thought success was just syntactic sugar to have four parameters (`data, status, headers, config`) instead of just `result`. Thanks for the help. – Kevin Salvesen Feb 26 '15 at 17:19
  • The task is simple. Keep the controller thin and in your example you are using only one $http request.Whereas in a project we might be writing more $http requests. So we could have a single $http requests in a factory or service and call it everytime you needed just by passing variables. It reduces your coding time and keeps controller thin. I dont see any other advantages. – Alaksandar Jesus Gene Feb 26 '15 at 17:21

3 Answers3

3

The data returned in the .then is available to the next chained .then handler, which is what you would ultimately use to get the data.

The .success just relays the original promise of $http.get. Returning data from .success doesn't do anything.

So, if you have:

 function getStuff() {
    return $http.get('/api/stuff')
        .success(function(data) {
            // do something with data. returning doesn't do anything
        })
        .error(function(data) {
            console.error(data);
        });
};

in the controller you would do:

getStuff().then(function(response){
  $scope.data = response.data; // this is the data available from `$http.get`
}
New Dev
  • 48,427
  • 12
  • 87
  • 129
  • Have you checked that `success` works like `then`? I'm quite sure of the opposite – Bergi Feb 26 '15 at 17:17
  • But success does not return a new promise, as demonstrated by the fact you can use a chained .error method after. – Dylan Watt Feb 26 '15 at 17:18
  • -1. In fact [`success` does not work like `then`](https://github.com/angular/angular.js/blob/master/src/ng/http.js#L814), and `return`ing from its callback is pointless. – Bergi Feb 26 '15 at 17:20
  • 1
    @Bergi, ah, true... I missed that. Returning from `.success` is pointless - it just relays the original `$http` promise. Fixed. – New Dev Feb 26 '15 at 17:26
  • I think there is a slight mistake in the code above. The `then` function should have a `result` parameter and not `data` since it is not equivalent to the `data` parameter of `success`. – Kevin Salvesen Feb 27 '15 at 10:34
  • @Klodo, yes - `data` is the response object in `.then`. The actual data would be `data.data`. I'll edit to make it clear, thanks for pointing it out – New Dev Feb 27 '15 at 13:54
1

The tutorial you refer to does not return the data in success handler, it returns result.data in the then handler. That then, which generates a new chained promise, is then returned. This removes the http request data normally present in .then.

Why bother?

The reason to work with .then instead of .success is for future proofing. .success is not part of promises, it's specific to $http. If you ever decide to later get the data from another async source (websockets, webworkers) your code will break when the promise you return no longer has a success handler.

By making it always return a .then, you make sure your services are sufficiently abstracted.

Dylan Watt
  • 3,357
  • 12
  • 16
  • Actually, the real reason of using `then` over `success` is that it creates the chained promise for the return value of the callback, which `success` doesn't. – Bergi Feb 26 '15 at 17:26
  • But there's no point in bothering with that if you're okay using `.success`, as the original promise given by `$http` has that method with the response data as it's only argument. The original question was asking why not bother just using `.success` in their controller. – Dylan Watt Feb 26 '15 at 17:39
  • Oh right, I referred to the `success` call in `MyStuffService.getStuff` not the one in the controller. – Bergi Feb 26 '15 at 17:46
  • 2
    Yeah, I was trying to explain why, in general, using .success is not a good choice. It's a little unfortunate those convenience methods exist, as it's confusing to people as they first try to learn promises, and `.success` and `.error` aren't part of it. – Dylan Watt Feb 26 '15 at 17:50
0

Normally you would do that if you need to do some processing before sending data to controller.

But in this case, I agree as there is no processing of data, there is no need to need to resolve the promise in the service.

In this case, I would simply rewrite the service method as:-

function getStuff() {
   return $http.get('/api/stuff');
}

If I have to do some processing before returning data to the controller, I tend to use $q, as below.

function getStuff() {
    var defer = $q.defer();
    $http.get('/api/stuff')
        .then(function(data) {
            // some processing
            defer.resolve(processedData);
        }, function(error) {
            defer.reject(error);   
        });
        return defer.promise;
}
Abhishek Jain
  • 2,957
  • 23
  • 35
  • 1
    [You really shouldn't do](http://stackoverflow.com/q/23803743/1048572). The return value of `$http.get` is a promise already, so just use `then`! – Bergi Feb 26 '15 at 17:21
  • Sorry, my bad... I copy-pasted OP's code and changed it.... Agreed, I should have changed it to then... – Abhishek Jain Feb 26 '15 at 17:22
  • 1
    No, I mean you should *use* `then` instead of creating a new deferred! Please read the post I've pointed to. – Bergi Feb 26 '15 at 17:24
  • Ahh, got you now..I partly agree to that...But if my service is accessed from multiple places and the data needs conditioning before use, I would use this pattern or probably httpInterceptor to transform my response. But as I said in my reply, if no processing is required, I will just return the promise. – Abhishek Jain Feb 26 '15 at 17:29
  • 1
    No, just don't use that pattern (ever)! If the data needs conditioning, you can and should use `then`. – Bergi Feb 26 '15 at 17:30