AngularJS - refactoring controller into service - understanding self/this and scope

Question

I'm new to AngularJS and my JavaScript knowledge right now isn't that strong, so I apologise in advance whilst I wear the white belt and expose my ignorance.

I'm having some difficulty refactoring the following controller into a service, primarily for seperation of concerns and pushing the logic further down the stack, and getting my controller skinny again.

airBnbClone.controller('SpacesCtrl', ['$http', '$location', function( $http, $location) {
  var self = this;
  self.spaces = [];
  self.currentspace;

  self.create = function(space) {
    $http.post('http://localhost:3000/api/spaces', space).success(function(data) {
      self.spaces.push(data);
      self.showSpace(data.space);
    });
  };

  self.getSpaces = function(){
    $http.get('http://localhost:3000/api/spaces.json').then(function(response){
      self.spaces = response.data.reverse();
    });
  };

  self.showSpace = function(space){
    $http.get('http://localhost:3000/api/spaces/' + space.id).then(function(response){
      self.currentspace = response.data;
      $location.path('/spaces/' + space.id)
    });
  };
}]);

After some refactoring, my code now looks like this:

airBnbClone.controller('SpacesCtrl', ['$http', '$location', 'spacesService', function( $http, $location, spacesService) {
  var self = this;

  self.create = function(space) {
    spacesService.createSpace(space);
  };

  self.getSpaces = function(){
    spacesService.getSpaces();
  };

  self.showSpace = function(space){
    spacesService.showSpace(space);
  };
}])
.service('spacesService', ['$http', '$location', function($http, $location){
  var self = this;
  self.spaces = [];
  self.currentspace;

  this.createSpace = function(space){
    $http.post('http://localhost:3000/api/spaces', space).success(function(data) {
      self.spaces.push(data);
      self.showSpace(data.space);
    });
  }

  this.getSpaces = function(){
    $http.get('http://localhost:3000/api/spaces.json').then(function(response){
      self.spaces = response.data.reverse();
    });
  };

  this.showSpace = function(space){
    $http.get('http://localhost:3000/api/spaces/' + space.id).then(function(response){
      self.currentspace = response.data;
      $location.path('/spaces/' + space.id)
    });
  };
}]);

Whereas before I refactored, my code was working as intended. My main view had an ng-init='spacescontroller.getSpaces()', with my ng-controller='SpacesCtrl as spacescontroller' pulling in my list of spaces. When I clicked on a particular space, it would then go to show that particular space as intended.

Now, with my refactored code my default view shows nothing at all, and when I do create a space, it seems like it can't update the original self.spaces array sitting in the controller.

Basically, I'm unsure of how to refactor these methods into services. Should my self.spaces and self.currentspace objects stay in the controller, or become properties of the injected service? Which is the preferred method for storing state in this case, and why?

Since my code doesn't render a view anymore, why is this the case? I apologise if my questions are quite circular, I've been going for days on this and despite consulting many different sources, I'm starting to feel very confused.

Answer 1

Consider returning promises from your service.

app.service('spacesService', function($http) {

    this.getSpaces = function(){
        var url = 'http://localhost:3000/api/spaces.json';
        var promise = $http.get(url)
            .then(function(response){
                //return for chaining
                return response.data.reverse();
        });
        return promise;
     });
});

The one of the advantages of returning promises is that they retain error information.

In your controller:

airBnbClone.controller('SpacesCtrl', function($http, $location, spacesService) {
    var self = this;
    self.spaces = [];

    self.getSpaces = function() {
        var promise = spacesService.getSpaces();

        promise.then(function onFulfilled(spaces) {
            self.spaces = spaces;
        }).catch(function onRejected(response) {
            console.log(response.status);
        });
    };
};

For more information on the advantages of using promises, see Why are Callbacks from Promise .then Methods an Anti-Pattern?.

Answer 2

You need a way to bring the results of the GET request from the service to the controller.

I left the logic in the controller and moved all the HTTP requests to the service. And I added a callback that will fire when the results are ready to let the controller make use of the results.

airBnbClone.controller('SpacesCtrl', ['$http', '$location', 'spacesService', function($http, $location, spacesService) {
        var self = this;
        self.spaces = [];
  self.currentspace;
        self.create = function(space) {
            spacesService.createSpace(space, function(data) {
                self.spaces.push(data);
                self.showSpace(data.space);
            });
        };

        self.getSpaces = function() {
            spacesService.getSpaces(function(spaces) {
                self.spaces = spaces;
            });
        };

        self.showSpace = function(space) {
            spacesService.showSpace(space, function(currentspace) {
                self.currentspace = currentspace;
                $location.path('/spaces/' + space.id)
            });
        };
    }])
    .service('spacesService', ['$http', function($http) {
        var self = this;


        this.createSpace = function(space, cb) {


            $http.post('http://localhost:3000/api/spaces', space).success(function(data) {
                cb(data);
            });
        }

        this.getSpaces = function(cb) {
            $http.get('http://localhost:3000/api/spaces.json').then(function(response) {
                var spaces = response.data.reverse();
                cb(spaces);
            });
        };

        this.showSpace = function(space, cb) {
            $http.get('http://localhost:3000/api/spaces/' + space.id).then(function(response) {
                var currentspace = response.data;
                cb(currentspace);
            });
        };
    }]);

Answer 3

The usage of service service assumes that a new object instance is injected that holds its own state. The typical use for that is an instance of model class ('model' like in MVC model).

The refactoring is on the right track. Considering that spacesService is a model, it can be just assigned to controller (instead of numerous wrappers for each of model's methods):

this.spacesService = spacesService;
// or any readable property name, e.g.
// this.spaces = spacesService;

So it could be reached from the view with

spacescontroller.spacesService.getSpaces()

The exception here is showSpace method. The part with $location.path has nothing to do with model state, and the fact that it does some routing indicates that it belongs to controller and shouldn't be extracted from it. So it can be separated to spacesService.getSpace(id) and showSpace in controller.