0

I would like to ask/discuss wether this is good or bad practise - what are the pros and cons of making a service call insde a controller as clean and short as possible. In other words: not having a callback anymore but make use of the Angular binding principles of Angular.

Take a look at a Plnkr I forked: http://plnkr.co/edit/zNwy8tNKG6DxAzBAclKY

I would like to achieve what is commented out on line 42 of the Plnkr > $scope.data = DataService.getData(3);.

app.factory('DataService', function($q, $http) {
      var cache = {};
      var service= {
        data:{src:''},
        getData: function(id, callback) {
          var deffered = $q.defer();
          if (cache[id]) {
            service.data.src = 'cache';
            deffered.resolve(cache[id])
          } else {
            $http.get('data.json').then(function(res) {
              service.data.src = 'ajax';
              cache[id] = res.data;
              cache[id].dataSource = service.data.src;
              deffered.resolve(cache[id])
            })
          }
          return deffered.promise.then(callback);
        }
      }
      return service
    })
     app.controller('MainCtrl', function($scope, DataService) {

        DataService.getData(3, function(result) {
          $scope.data = result;
        });

     //$scope.data = DataService.getData(3);
});
San Jay Falcon
  • 993
  • 1
  • 9
  • 20

3 Answers3

3

My best practice with regards to services requesting data and returning promises is:

  • return a promise (in DataService, return deferred.promise)
  • in the controller, call DataService.getData(3).then(, )

So I would not pass a callback to a service function that uses a promise.

The more difficult question is what should the service function do, and what should the then(function(data) {...}) do. Here are a few guidelines:

  • Anything that is shared (data / repetitive functionality), implement in the service
  • Anything related to binding data / functions to UI elements, implement in the controller
  • Keep your controllers as simple as possible - they should just link between UI elements and the model.
  • Any model logic, processing, format parsing, etc - implement in the service

I added this part after reading the comments:

If you need to do some processing (like checking a cached result, parsing the result, etc.), then the proper place to do this is in the service.

So I would modify your code as follows:

var service= {
  data:{src:''},
  getData: function(id) {
    var deffered = $q.defer();
    if (cache[id]) {
      service.data.src = 'cache';
      deffered.resolve(cache[id]);
      return deferred;
    } else {
      return $http.get('data.json').then(function(res) {
        service.data.src = 'ajax';
        cache[id] = res.data;
        cache[id].dataSource = service.data.src;
        return cache[id]; // this will resolve to the next ".then(..)"
      });
    }
  }
}
Rami Amar
  • 46
  • 3
  • 1
    another thing you would want to implement in the controller, is handling errors of the DataService. Usually you would want to tell the user about the error ( == link data to a ui element), and that should be done in the controller. So that's a pro in favor of returning the promise (and not passing a callback). – Rami Amar Jun 17 '15 at 14:18
  • gonna accept this awnser as the one that guides me in the right direction. some personal feedback on UI manipulation: dont want this in controllers, that stuff needs to be handled by directives (as much as possible). – San Jay Falcon Jun 18 '15 at 09:07
  • still one question remains and that is about chaining promises, i would rather see this being done in the service then the controller before the later gets a mess. whats your thought on that? – San Jay Falcon Jun 18 '15 at 09:09
  • I agree - if you have some processing to do on the $http response, then handle it in the service, and then just make sure you return the result (and it will resolve into the next chained promise). Also make sure to handle errors, so they pass on to the next promise handler. – Rami Amar Jun 19 '15 at 16:09
1

AfaIk this is not possible - see this Thread

You can 'automatically' resolve the promise by using angular-route. The resolved promises will be injected into your controller.

Community
  • 1
  • 1
Michael
  • 3,085
  • 1
  • 17
  • 15
  • good one, and i like the (clean) solution, however this is a service that can be called at runtime, not when building a view initialy. good point though. – San Jay Falcon Jun 18 '15 at 09:05
  • the angular-route states are defined in the config lifecycle, but the promises will be resovled at runtime – Michael Jun 18 '15 at 12:44
  • i mean calling the service when a button is clicked, from the UI for instance. the angular-route doesnt help me with that right? – San Jay Falcon Jun 18 '15 at 16:29
-1

You can do like this plunker

<!DOCTYPE html>
<html ng-app="plunker">

<head>
  <meta charset="utf-8" />
  <title>AngularJS Plunker</title>
  <script>
    document.write('<base href="' + document.location + '" />');
  </script>
  <link href="style.css" rel="stylesheet" />
  <script data-semver="1.2.4" src="http://code.angularjs.org/1.2.4/angular.js" data-require="angular.js@1.2.x"></script>
  <script src="app.js"></script>
  <script>
    app.factory('DataService', function($q, $http) {
      var cache = {};
      var service= {
        data:{src:''},
        getData: function(id, callback) {
          var deffered = $q.defer();
          if (cache[id]) {
            service.data.src = 'cache';
            deffered.resolve(cache[id])
          } else {
            $http.get('data.json').then(function(res) {
              service.data.src = 'ajax';
              cache[id] = res.data;
              cache[id].dataSource = service.data.src;
              deffered.resolve(cache[id])
            })
          }
          return deffered.promise;
        }
      }
      return service
    })
     app.controller('MainCtrl', function($scope, DataService) {

         DataService.getData(3).then(function (data) {
          $scope.data = data;
        });
    });
  </script>
</head>

<body ng-controller="MainCtrl">

<div>Source: {{data.dataSource}}</div>
  <pre>{{data}}</pre>
</body>

</html>
simon
  • 1,415
  • 10
  • 20