Resolving a promise in a service/factory vs in a controller with AngularJS

Question

So I have played around with getting a promise to resolve in a service vs in a controller. I'd prefer to resolve it in the service so I can reuse the variable without having to resolve it multiple times.

The problem I'm having is that it works, but it's returning the data very very slowly. So I feel like I'm doing something wrong here. it takes about 5 or 6 seconds for my ng-options to populate. Which is better? And how can I improve my code so it runs faster?

Resolved In Service:

resortModule.factory('locaService',['$http', '$rootScope', function ($http, $rootScope){
    locaService.getLocations=
        function() {
            return $http.get('/api/destinations').then(
                function(result){
                    locaService.locations= result.data;
                    return locaService.locations;
                }
            );
        return locaService.locations;
    };
resortModule.controller('queryController',['$scope', 'locaService', function($scope, locaService) {
    $scope.getLocations= locaService.getLocations().then(function(result){
       $scope.locations= result;
    });
}]);

Resolved in Controller:

resortModule.factory('locaService',['$http', '$rootScope', function ($http, $rootScope){
locaService.getLocations=
    function() {
        locaService.locations= $http.get('/api/destinations');
        //stores variable for later use
        return locaService.locations;
    };
}]);
resortModule.controller('queryController',['$scope', 'locaService',          
    function($scope, locaService) {
       locaService.getLocations()
       .then(
            function(locations) // $http returned a successful result
            {$scope.locations = locations;} //set locations to returned data
       ,function(err){console.log(err)});
}]);

HTML:

<select ng-click="selectCheck(); hideStyle={display:'none'}" name="destination" ng-style="validStyle" ng-change="getResorts(userLocation); redirect(userLocation)" class="g-input" id="location" ng-model="userLocation">
    <option value=''>Select Location</option> 
    <option value='/destinations'>All</option>
    <option value="{{loca.id}}" ng-repeat="loca in locations | orderBy: 'name'">{{loca.name}}</option>
</select>

Saw

Caching the promise is much simpler than caching the value. – Mucky

@Mucky thanks for the link. – Alver

Answer 1

In angular, services are singletons so there is only one instance in your app. This allows you to resolve data once (in your service), store it, and then on subsequent calls just return the already resolved data. This will allow you to not resolve your data multiple times and also keep your logic separated between service and controller.

UPDATE - cache promise instead, thanks yvesmancera for the bug find

resortModule.factory('locaService', ['$http', '$rootScope', function ($http, $rootScope) {
    var locationsPromise = null;

    locaService.getLocations =
        function() {
            if (locationsPromise == null) {
                locationsPromise = $http.get('/api/destinations').then(
                  function(result) {
                      return result.data;
                  }
                );
            }

            return locationsPromise;
        };

    ...
}

resortModule.controller('queryController',['$scope', 'locaService', function($scope, locaService) {
    $scope.getLocations= locaService.getLocations().then(function(result) {
        $scope.locations= result;
    });
}]);

As far as speeding up your loading of data, I don't see anything wrong with your javascript. It could just be your api call taking up a lot of time. If you post your HTML related code we could check that out and see if anything could be slowing it down.

Alver

Your code has bugs though, on subsequent calls you're no longer returning a promise, so getLocations().then would fail. – Jorgenson

@Alver I do think your code looks better. I added the html to the original question. I use an ng-repeat so I can have the 'all' option that redirects to a different page. Otherwise I would have used the ng-options. Just so you're aware. – Saw

@BrookeClonts ng-options does provide speed optimizations over ng-repeat so that could be causing the delay – Alver

Okay, I'll try it with the ng-options. It's not worth having an 'all' option if it's going to delay the page that much. Thanks so much! You guys are the best. Sorry, I'm learning as I go along. – Saw