Keep angular controller thin

Question

At this moment i working on huge angular SPA application. I try to keep my controllers thin:

<div ng-controller='HomeController as home'>
   <div ng-repeat='var item in home.items' ng-bind='item' ></div>
   <button ng-click='home.remove(1)' >remove</button>
</div>

function HomeController (homeService){
    var vm = this; //$scope
    vm.items = [1,2,3,4,5];
    vm.remove = remove;

    function remove (id){
        homeService.remove({ items: vm.items, targetId: id });
    }

    //a lot of other logic here
}


angular.module('my').service('homeService', homeService);
function homeService (){
    this.remove = remove;

    function remove (param){
        for(var a = 0; a < param.items.length; a++){
            if(param.items[a] == param.targetId){
                param.items.splice(a, 1);
                break;
            }
        }
    }
}

Advantages:

• Logic is outside of controller

Disadvantages:

• Service change scope state

What is your approach to keep controllers thin?

Answer 1

What is your approach to keep controllers thin?

I personally like to keep anything related to application models inside factories/services. So remove and item in your code would not be defined in the controller. Inside the controller I would set references to the model for whatever needs to be available to directives i.e. accessible via $scope.

As an example, consider a model with an array of entities and methods to add/remove/find entities in the array. I would first create a factory exposing my model and methods to work with it:

angular.module('myApp').factory('model', function() {

    // private helpers
    var add = function(array, element) {...}
    var remove = function(array, element) {...}
    var find = function(array, id) {...}

    return {
        Entity: function(id) {
            this.id = id;
        },
        entities: {
            entities: [],
            find: function(id) {
                return find(this.entities, id);
            },
            add: function(entity) {
                add(this.entities, entity);
            },
            remove: function(entity) {
                remove(this.entities, entity);
            }       
        }
});

Then pass the model to my controller:

angular.module('myApp').controller('ctrl', function($scope, model) {
    $scope["model"] = model; // set reference to the model if i have to
    var entity = new model.Entity('foo'); // create a new Entity
    model.entities.add(entity); // add entity to entities
    model.entities.find('foo'); // find entity with id 'foo'
});

etc.

Answer 2

The first thing that I miss in your example are directives. Directives are a powerfull Angular tool that allows you to reuse code, encapsulate and expose the behaviour in your html. To keep the controllers thin you need to split your logic in directives and services. I would write your example with something like: (not working code, I wrote something to ilustrate the idea of splitting the logic)

// "home"
<div ng-controller='HomeController as home'>
   <my-item ng-repeat='var item in home.items' ng-bind='item'></my-item>
</div>

// itemtemplate.html
<div>
    {{ item.name }}
    <button ng-click='remove()' >remove</button>
</div>

function HomeController (homeService){
    var vm = this; //$scope
    vm.items = homeService.items;

    // The idea here is to make this controller 
    // only needed to load content for this route, 
    // all the other logic should be in the directives and services 
}

angular.module('my').directive('myItem', funcion(homeService){
    return {
        restrict: 'E',
        templateUrl: 'itemtemplate.html',
        controller: function(scope, element, attrs) {
            scope.remove = function(){
                homeService.remove(scope.item);
            }
        } 
    }  
})

angular.module('my').factory('homeService', homeService);
function homeService (){

    var items = [];

    return { 
        loadItems: function() {
            items = [... pick the items from server or whatever ...];
        },

        remove: function() {
            for(var a = 0; a < this.items.length; a++){
                if(this.items[a] == this.targetId){
                    this.items.splice(a, 1);
                    break;
                }
            }
        }
    };
}

I also moved the items array to the service (I also changed it for a factory as I think services get instantiated with every injection and you need to keep the items available to everyone), this way it can be accessed from other controllers or directives and the controller doesn't have to care about it.