Why are Callbacks from Promise `.then` Methods an Anti-Pattern

Question

I have seen answers on StackOverflow where people suggest furnishing a callback function to an AngularJS service.

app.controller('tokenCtrl', function($scope, tokenService) {
    tokenService.getTokens(function callbackFn(tokens) {
        $scope.tokens = tokens;
    });
});

app.factory('tokenService', function($http) {
    var getTokens = function(callbackFn) {
        $http.get('/api/tokens').then (function onFulfilled(response) {
            callbackFn(response.data);
        });
    };

    return {
        getTokens: getTokens
    };
});

This seems to me to be an Anti-Pattern. The $http service returns promises and having .then methods execute callback functions feels like an unhealthy inversion of control.

How does one re-factor code like this and how does one explain why the original way was not a good idea?

Farm

Just tell people to actually use promises instead of ignoring them. – Geoid

I found using promise is kind of healthy, as it has ability to chain it and have control over async call. – Linkage

Main issue is that with nothing returned in then() there is nowhere to catch errors in callback. It breaks the promise chain – Alveraalverez

using the promise makes the code shorter and simpler too. – Sciatic

this service returning an object doesn't make much sense... it needs to return a promise so that whatever uses it can know when the data is ready. – Sciatic

"these callbacks from a promise are a big anti-pattern — architects hate them!" – Bois

Answer 1

The code can be re-factored as follows:

app.controller('tokenCtrl', function($scope, tokenService) {
    tokenService.getTokens.then ( callbackFn(tokens) {
        $scope.tokens = tokens;
    });
});

app.factory('tokenService', function($http) {
    var getTokens = function() {
        //return promise
        return $http.get('/api/tokens').then (function onFulfilled(response) {
                //return tokens
                return response.data;
            }
        );
    };

    return {
        getTokens: getTokens
    };
});

By having the service return a promise, and using the .then method of the promise, the same functionality is achieved with the following benefits:

• The promise can be saved and used for chaining.

• The promise can be saved and used to avoid repeating the same $http call.

• Error information is retained and can be retrieved with the .catch method.

• The promise can be forwarded to other clients.

Farm

Answer 2

You should change it to

var getTokens = function() {
      return $http.get('/api/tokens');
    };

And, then in other module use

yourModule.getTokens()
  .then(function(response) {
    // handle it
  });

As to why it's an anti-pattern, I'd say that, first, it doesn't allow you to further chain your success/fail handler methods. Second, it handles the control of processing the response from caller-module to called module (which might not be super-important here, but it still imposes same inversion of control). And finally, you add the concept of promises to your codebase, which might not be so easy to understand for some of the teammates, but then use promises as callbacks, so this really makes no sense.

Fairminded