Fix minor errors in code samples

[anthonychu]

Just a couple of minor fixes...

vm.refresh = dataservice.refresh(); should probably be vm.refresh = dataservice.refresh;

$http's then() passes its success and error callbacks a response object. The data is in the response object's data property.

[johnpapa]

The first you are correct. Nice catch.

The second does work as shown, its direct from my code. If you want to revise the PR, I'll merge

[anthonychu]

Are you using it it with then() or success()? success() passes (data, status, headers, config, statusText) as separate arguments to its callback, but then() passes its callbacks a single response object with data, status, headers, config, statusText properties.

[johnpapa]

Yep, correct. Thanks for the second set of eyes. I had switched form success to then and didn't update the code.

        function getAvengers() {
            return $http.get('/api/maa')
                .then(getAvengersComplete)
                .catch(function(message) {
                    exception.catcher('XHR Failed for getAvengers')(message);
                    $location.url('/');
                });

            function getAvengersComplete(response) {
                return response.data[0].data.results;
            }
        }

[anthonychu]

Awesome. Thanks for putting this guide together. :)

Not sure how picky we should be with the examples but then(getAvengersComplete).catch(getAvengersFailed) adds an extra promise to the chain and is better as then(getAvengersComplete, getAvengersFailed).

This example also reminds me of a very common anti-pattern that might be worth mentioning in the style guide, which is to return the promise from $http or $resource from a data service method...

function dataservice($http, logger) {
    return {
        getAvengers: getAvengers
    };

    function getAvengers() {
        return $http.get('/api/maa');
    }
}

The calls to the service end up either dealing with the $http response object in then()...

dataservice.getAvengers()
    .then(function(response) {
        vm.avengers = response.data;
    });

Or they use the $http-specific success() callback...

dataservice.getAvengers()
    .success(function(data) {
        vm.avengers = data;
    });

Both leak the underlying $http implementation detail to the caller.

[johnpapa]

Good questions.

Regarding the then(yay).catch(boo) vs then(yay, boo) ...

First, I prefer showing the then and the catch separately, for clarity. So that part is a preference.

Second, the then(getAvengersComplete).catch(getAvengersFailed) will handle an error even if it happens in the then(getAvengersComplete). With the then(getAvengersComplete, getAvengersFailed) if an error occurs in the getAvengerComplete, then it won't be handled by the getAvengersFailed.

In short, using .then(yay).catch(boo); catches more.

Regarding the $http examples you mention ...

I agree that returning the $http straight up as $http.get('/api/foo'); is not ideal. It leaves the caller to handle what to do next. I like to take the next step inside the dataservice and grab the data I need, possibly re-structure or strip out what I need, and then return that in a promise.

I'll think about how to articulate that in the guide, if I decide to add it.

-- Thanks to @wardbell for his input on this, as he is the one who pointed it out to me.

[anthonychu]

Excellent point on why .then(yay).catch(boo) is better. Thanks!

[wardbell]

John and I were just talking about this a minute ago. We like catch(boo) if boo can easily handle both the $http failure and an exception in yay. If it can't or that is too awkward AND you think a yay exception is a serious possibility then you can roll with then(yay, boo).catch(wtf)

We could really be splitting hairs here.

p.s.: there is no harm in having an extra promise in the chain.

[johnpapa]

good question and chat ... next time put it in its own issue so folks can discover it :)