0

Over the time in using angular i have switched from one practive of saving state and data to another here are the two.
Please suggest me which one to continue saving and why?

     angular.module('app')
          .controller('personController', ['$scope', '$log',personsService, cntrl])
          .factory('personsService', ['$http', '$q', service]);

//First
        function cntrl($scope, $log, service){
          $scope.state = {creating:false; updating:false;}
          $scope.createPerson = createPerson;
          $scope.person = null;
          function createPerson(fn, ln, age){
            $scope.state.creating = true;
            service.createPerson({fn:fn, ln:ln, age:age})
              .success(function(r){/*something*/})
              .error(function(){/*something*/})
              .finally(function(){
                 $scope.state.creating = true;
               });
          }
        }
        function service($http, $q){
            var r = {
                createPerson : createPerson
            };
            return r;

            function createPerson(o){
                return $http.post('/person/create', o);
            }
        }

//Second
        function cntrl($scope, $log, service){
            $scope.person = service.person;
            $scope.state = service.state;
            $scope.service = service;
        }

        function service($http, $q){
            var state = {creating:false, updating:false};
            var person = {};
            var r = {
                state : state, 
                person : person,
                createPerson : createPerson
            }

            return r;

            function createPerson(o){
                state.creating = true;
                var def = $q.defer();
                $http.post('/person/create', o)
                    .success(function(dbPerson){
                        def.resolve(dbPerson);
                        angular.extend(dbPerson, person);
                    })
                    .error(function(e){
                        def.rreject(e);
                        angular.extend({}, person); //or any other logic
                    })
                    .finally(function(){
                        state.creating = false;
                    });

                return def.promise;
            }

        }

As you can see in the
1. first example The ajax state is maintained in the controller. and service only exposes functions that are required for that ajax state. The Person object is also maintained inside the controller so i dont have to maintain the reference to the same object as the object is directly attached to the cart. I can simpply do $scope.person = {};

2. Second Example The ajax state is maintained by the service which now exposes the person object as well as the functions used to manipulate the object. Now i need to maintain the reference to object bby using functions sucn as angular.extend and angular.copy. Now the ajax state exposed by a controller is divided over multiple services. Advantages are modularity of the code and almost complete logic less controller.

Parv Sharma
  • 12,581
  • 4
  • 48
  • 80
  • Don't wrap existing promises into artificial deferreds as in example 1 service! Do return the promise that's the way it's intended to be used – Kirill Slatin Jul 01 '15 at 09:01

2 Answers2

1

I would suggest you to Go for the second Method. An AJAX Call for data should always be separate from Controller . Controllers require data that should be shown in the UI. Tomorrow you might require the same data from the same API to be used in a different way . In this way you are using the same service to get the data and modify according to the need . More over the Controller should not be worried about where the data is coming from. i.e it could come from An AJAX call , a web socket, local storage etc.

So in your case angular.extend makes more sense as you are working on a copy of the data and not the original data which might be used by some other controller over time. Re-usable and clean.

  • In both of the cases the ajax call is separate from the controller. The service only returns a promise the structure of the promise varies. Would you recommend the first approach if i return a $q promise instead of $http promise? – Parv Sharma Jun 25 '15 at 11:59
  • moreover since the data is retrieved from the service. it can done in any controller in inject the service. – Parv Sharma Jun 25 '15 at 12:00
  • The state variable is indeed used in UI states like loading and refreshing so should be in the controller therefore case 1? – Parv Sharma Jun 25 '15 at 12:01
  • 1
    Well, if you have the only controller that deals with `state` in UI, then who cares? However if an app becomes bigger, can you guarantee that same `state` is not used in several controllers or directives? Thus approach 2 when angular `service` not only maintains requests but also handles current state of this job is more proper. – Kirill Slatin Jul 01 '15 at 08:59
0

I would say the correct approach would be the mix of both the approaches. @Parv mentioned the state object is being used at the UI, it should rest in teh controller. But the service function in approach 1 is at fault. The logic there should use the $q defer object and be something like what you are doing in your approach 2..

 angular.module('app')
          .controller('personController', ['$scope', '$log',personsService, cntrl])
          .factory('personsService', ['$http', '$q', service]);

//First
        function cntrl($scope, $log, service){
          $scope.state = {creating:false; updating:false;}
          $scope.createPerson = createPerson;
          $scope.person = null;
          function createPerson(fn, ln, age){ 
            $scope.state.creating = true;          
            service.createPerson({fn:fn, ln:ln, age:age})
              .then(function(r){/*assign $scope.person = r;*/}),function() 
               {/*something*/}).finally(function() {
                $scope.state.creating = false;
              });
          }
        }

        function service($http, $q){
            var r = {
                createPerson : createPerson,
                person: {}
            };
            return r;

            function createPerson(o){                  
              var def = $q.defer();
              $http.post('/person/create', o)
                 .success(function(dbPerson){
                        def.resolve(dbPerson);
                        angular.extend(dbPerson, person);
                    })
                    .error(function(e){
                        def.reject(e);
                        angular.extend({}, person); //or any other logic
                    });

                return def.promise;
            }
        }

This way we have segregated the post logic from the things that matter at the UI.

  • It's a very bad practice to wrap existing promise into an artificial deferred! Just do `return $http.post().except().finally();` – Kirill Slatin Jul 01 '15 at 08:56