1

In my Angular application, I have two controllers which both need access to the same data.

Toward that end, I've created a service which will be responsible for holding and providing access to that data:

angular.module("SomeModule").factory( "SomeService", function( $http ) {

    var svc = {};
    var data = {};

    // on initialization, load data from the server
    $http.get( "somefile.json" )
        .success( function( data ) {
            svc.data = data;
        } );

    svc.getItem = function( id ) {
        // find the specified item within svc.data, and return its value
    };

    return svc;

} );

...and I've injected that service into each of the two controllers:

angular.module("SomeModule").controller( "SomeController", function( $routeParams, SomeService ) {

    var ctrl = this;

    ctrl.item = null; // set an initial value

    // load the item that was requested in the URL
    ctrl.item = SomeService.getItem( $routeParams.id );

} );

This almost works - but it has one big flaw. If SomeController calls SomeService.getItem() before SomeService finishes loading somefile.json, then SomeService won't have any data to return.

In practice, if I load the app a few times, some loads will work (i.e., SomeService will finish loading somefile.json first, and the controller will present the data as desired), and other loads don't (i.e., SomeController will try to retrieve data from SomeService before the data has actually been loaded, and everything will crash and burn).

Obviously, I need to find some way to defer the execution of getItem() until SomeService is actually ready to process those calls. But I'm not sure of the best way to do that.

I can think of a some rather hairy solutions, such as building my own call queue in SomeService, and wiring up a bunch of complicated callbacks. But there's gotta be a more elegant solution.

I suspect that Angular's $q service could be useful here. However, I'm new to promises, and I'm not sure exactly how I should use $q here (or even whether I'm barking up the right tree).

Can you nudge me in the right direction? I'd be super grateful.

greenie2600
  • 1,679
  • 3
  • 17
  • 24
  • Just make `getItem` return a promise - and store a promise for the data in `svc.data`, not the "data when it's ready". – Bergi May 21 '15 at 14:11
  • As @Bergi said, you have to return a promise. You have some exemple in my answer, check the 2nd important point if you want details about this – Okazari May 21 '15 at 14:35
  • as @Bergi suggested i adapted the exemple in my answer to fit yours. – Okazari May 21 '15 at 14:45

5 Answers5

2

I would recommend making better use of AngularJS' routing capabilities, which allow you to resolve dependencies, along with the $http services cache, and structuring your application accordingly.

I think you need to, therefore, get rid of your service completely.

Starting with the example below, taken straight from the Angular documentation:

phonecatApp.config(['$routeProvider',
  function($routeProvider) {
    $routeProvider.
      when('/phones', {
        templateUrl: 'partials/phone-list.html',
        controller: 'PhoneListCtrl'
      }).
      when('/phones/:phoneId', {
        templateUrl: 'partials/phone-detail.html',
        controller: 'PhoneDetailCtrl'
      }).
      otherwise({
        redirectTo: '/phones'
      });
  }]);

So PhoneListCtrl and PhoneDetailCtrl both need the data from somefile.json. I would inject that data into each controller like so:

(function(){
        angular.module('phonecatApp').controller('PhoneListCtrl', ['somefileJsonData', function(somefileJsonData){
            this.someFileJsonData = someFileJsonData;
        }]);
})();

The same idea for PhoneDetailCtrl.

Then update your routing like so:

phonecatApp.config(['$routeProvider',
  function($routeProvider) {
    $routeProvider.
      when('/phones', {
        templateUrl: 'partials/phone-list.html',
        controller: 'PhoneListCtrl',
        resolve:{ somefileJsonData: ['$http',function($http){
            return $http.get("somefile.json", { cache: true });
        }] }
      }).
      when('/phones/:phoneId', {
        templateUrl: 'partials/phone-detail.html',
        controller: 'PhoneDetailCtrl',
        //same resolve
      }).
      otherwise({
        redirectTo: '/phones'
      });
  }]);

This way, you are letting angular take care of resolving this dependency as part of the routing process.

Setting cache to true will also cache it so you aren't doing the same Get request twice, and Angular will only show your view when the dependency is resolved.

So, in your app, where SomeController is paired with a view as part of the routing process, use resolve to resolve item, and inject this into the controller.

JMK
  • 27,273
  • 52
  • 163
  • 280
  • In my opinion what you are saying is true but services are also intended to manage shared collections of object within your module. – Okazari May 21 '15 at 14:29
  • Hmm. I like the spirit of this, but what actually gets injected into my controller is a complex object with properties named `data` (this contains the loaded JSON), `status`, `config`, and `statusText`. Obviously I could just adjust the controller to say `this.someFileJsonData = someFileJsonData.data` instead of `this.someFileJsonData = someFileJsonData`, but that feels messy - it couples the controller to the implementation of someFileJsonData. Is there a way to clean up the `resolve` in the route definition so that it injects just the loaded JSON data? – greenie2600 May 21 '15 at 15:00
  • Also, the Angular doc page you linked to doesn't mention `resolve` anywhere. – greenie2600 May 21 '15 at 15:01
  • 1
    @greenie2600 You also can use your Service in the resolve instead of $http if you need it (but it has to return a promise !). According to your usecase, JMK solution is the best way to go. – Okazari May 21 '15 at 15:13
  • More info on resolve [here](https://docs.angularjs.org/api/ngRoute/provider/$routeProvider), also yeah you may need to access a property of the object being passed in, I find it's a small price to pay for much tidier code though – JMK May 21 '15 at 15:14
  • Also, status, config and statusText sound useful, are you sure you want to discard them? – JMK May 21 '15 at 15:32
  • They *are* useful - to the code that's responsible for loading the data. Pushing responsibility for interpreting the results of the HTTP GET into the controller is messy - the controller shouldn't have to care about those details; it only needs the data. The controller shouldn't even have to *know* that the data was retrieved via HTTP - as far as the controller is concerned, the data could have been fetched from a cookie, or read from user input, or calculated on the fly. – greenie2600 May 21 '15 at 15:40
  • At least that's my instinct - maybe you're right, and the benefits *are* worth this mixing of concerns. I do appreciate your answer - it's good to know that this kind of thing is possible with Angular, and I intend to explore it further. So, thanks. – greenie2600 May 21 '15 at 15:40
2

try this code

angular.module("SomeModule").factory("SomeService", function ($http) {
    var svc = {};

    svc.getList = function () {
       return $http.get("somefile.json");
    };

    svc.getItem = function (id) {
        svc.getList().then(function (response) {
            // find the specified item within response, and return its value
        });
    };
    return svc;
});
z0r0
  • 666
  • 4
  • 14
  • Your `deferred` is initialised in the wrong scope (and of course [you shouldn't use deferreds](http://stackoverflow.com/q/23803743/1048572) at all) – Bergi May 21 '15 at 14:50
  • @Bergi, I removed the usage of $q, but I think no need to return the promise to the controller, because the value of item in the controller will be updated automatically ( 2 ways data binding) – z0r0 May 21 '15 at 15:52
  • But there is no data binding established anywhere? – Bergi May 21 '15 at 15:56
1

Here is how i did it in my own project.

Your Service

angular.module("SomeModule").factory( "SomeService", function( $http ) {

    var svc = {};
    svc.data = {};

    // on initialization, load data from the server
    svc.getData = function(){
        return $http.get( "somefile.json" );
    };

    return svc;

} );

Your Controllers

angular.module("SomeModule").controller( "SomeController", function( $routeParams, SomeService ) {

    ctrl.items = null; // set an initial value

    // load the item that was requested in the URL
    SomeService.getData().success(function(data){
        ctrl.items = data;
    }).error(function(response){
        console.err("damn");
    });

} );

Important point : Promises

In my humble opinion, the responsibility for processing asynchronous call is due to the controller. I always return a $http promiss whenever i can.

 svc.getData = function(){
    return $http.get( "somefile.json" );
 };

You can add some logic in your service but you always have to return the promise. (To know : .success() on a promise return the promise)

The controller will have the logic to know how to behave depending to the response of your asynchronous call. He MUST know how to behave in case of success and in case of error.

If you have more question feel free to ask. I hope it helped you.

Okazari
  • 4,597
  • 1
  • 15
  • 27
  • Maybe you can show a solution to the OP's question, and not how you've solved your own project? – Bergi May 21 '15 at 14:36
  • @Bergi Totally true, i'll do this now – Okazari May 21 '15 at 14:38
  • In fact `.success()` doesn't return a new but the original promise. And it shouldn't be used here at all. – Bergi May 21 '15 at 14:52
  • @Bergi Oh ? Maybe a mistake i'm doing then. I felt that i could wrap my "service" handling of the ressource in the .success() of the response then return it to the controller. The controller then do his own handling of the response. Am i wrong ? If so i'll edit this – Okazari May 21 '15 at 14:58
  • Yes, returning the promise to the controller is correct. But `.success(function(data) { svc.data = data; })` is quite useless – Bergi May 21 '15 at 15:03
  • @Bergi in my use case it had some sense but not in this one, you're correct i'll edit it. – Okazari May 21 '15 at 15:06
  • I tried a couple of approaches suggested in the answers, and this one seemed to be the cleanest. It also helps me understand promises better - I had already been consuming them (*e.g.*, when calling `$http.get()`), but this is my first experience with returning my own promises. Thanks to both of you! – greenie2600 May 21 '15 at 15:27
0

ther are 2 good options you can

  1. use callback

  2. use $q return promise

Using Callback:

svc.getItem = function( id,callback ) {
      $http.get( "somefile.json" )
        .success( function( data ) {
            svc.data = data;
            callback(svc.data)
        } );
    };

in controller

SomeService.getItem( $routeParams.id,function(data){
 ctrl.item =  data
 } );

Using Promise:

  svc.getItem = function( id) {

   var deferred = $q.defer();

      $http.get( "somefile.json" )
        .success( function( data ) {
            svc.data = data;
            deferred.resolve(svc.data);
        } )
        .error(function (error) {
        deferred.reject(error);
         });

      return deferred.promise;
     ;
    };

in controller

SomeService.getItem( $routeParams.id).then(function (data) {
     ctrl.item =  data
},
function (error) {
    //do something with error
});
A.B
  • 20,110
  • 3
  • 37
  • 71
0

Here's how we do it, we use $q to defer whatever the async call will provide your service, I then take the data part of the response and resolve it, it sends the required data to the controller (without status, headers...).

I use a try catch statement in my service, to keep error handling away from the controller.

angular.module("JobsService", [])
    .factory("JobsService", ['$q', '$http', '$log', function ($q, $http, $log) {

        var serviceName = "JobsService";
        var url = "http://localhost:8080/path/to/resource/";

        var service = {};

        service.getAll = function () {

            var deferred = $q.defer();

            try {
                $http.get(url + "/jobs")
                        .success(function (response, status) {
                            $log.debug("GET response in " + serviceName + " returned with status " + status);
                            deferred.resolve(response);
                        })
                        .error(function (error, status) {
                            deferred.reject(error + " : " + status);
                        });
            } catch (err) {
                $log.error(err);
                deferred.reject();
            }

            return deferred.promise;
        };

        return service;
    }]);

then in controller

JobsService.getAll()
         .then(function (response) {

             $scope.jobs = response;
             // records are stored in $scope.jobs
         }, function (response) {
             $scope.jobs = undefined;
         })
             .finally(function () {
              // will always run
         });
svarog
  • 9,477
  • 4
  • 61
  • 77
  • Don't use the [deferred antipattern](http://stackoverflow.com/q/23803743/1048572)! – Bergi May 21 '15 at 14:14
  • but all the cool kids do it. actually you are correct, i only do it because it helps to keep my controllers a bit tidier – svarog May 21 '15 at 14:18
  • All the cool kids use promises. The deferred antipattern is about the mess in your factory, the controller should be as tidy as you have written it. – Bergi May 21 '15 at 14:20