current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 1 down vote favorite

I have a project built with AngularJS and using Restangular in my services to handle API requests. The problem is I haven't found a good way to test the services which I think has turned into a bit of a code smell for me. I'm using callbacks in the promise to handle pushing the data back to the controller instead of just returning the result.

Here is a simple controller and service:

myApp.controller('CustomerController', ['$scope', 'Teams', function($scope, Teams) {
    Teams.getList(function(results) {
        $scope.teams = result;
    });
}]);
myApp.factory('Teams', ['Restangular', function(Restangular) {
    var Teams = {};
    Teams.getList = function(callback) {
        Restangular.all('teams').getList().then(function(results) {
            callback(results);
        });
    };
    return Teams;
}]);

Is that really the best way to get the data back to the controller? I'm starting to feel like this isn't the correct way to do it but I haven't found another way that works.

share|improve this question
up vote 3 down vote accepted

Well, when you're already using promises you also just return that instead of using a callback. Then you can also chain more promises at the end and add error handling. Other than that it looks fine; I don't see what you'd really change there.

myApp.controller('CustomerController', ['$scope', 'Teams', function($scope, Teams) {
    Teams.getList().then(function(results) {
        $scope.teams = result;
    }, function(error) {
        ...
    });
}]);
myApp.factory('Teams', ['Restangular', function(Restangular) {
    var Teams = {};
    Teams.getList = function() {
        return Restangular.all('teams').getList();
    };
    return Teams;
}]);

If you want to get rid of Teams, I've seen this variant as well:

myApp.factory('Teams', ['Restangular', function(Restangular) {
    var getList = function() {
        return Restangular.all('teams').getList();
    };

    return {
        getList: getList
    };
}]);
share|improve this answer
    
So you don't see an issue with the way the service is setup? The fact that Teams doesn't really seem to return anything has been bugging me but I wasn't sure why since there doesn't seem to be any advantage to changing. Also, the point about error handling is good. – Matthew Green Oct 7 '14 at 20:56
    
No, not really. I mean you won't get around the async call, so there are only so many ways to do this. I'm adding a slight rewrite, but that's just stylistic. – ferada Oct 7 '14 at 21:02

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.