AngularJS Todo List - too much controller logic?

I am starting an angular app and I was wondering if I am on the right track with this. I worked on my last angular app when I was just a youngun who didn't understand the important of designing the code properly. I failed so hard at keeping the code clean and maintainable, mostly due to not understanding directives or services.

I am wondering if there is a better structure to my error handling in the below code. Is that too much logic in the controller functions? All I am doing is allowing the adding of items to a list, but not allowing them to be added or removed if the list length is 1 respectively. If there are other problems besides the controller logic, please let me know.

var toDo = angular.module('toDo',[]);

toDo.factory('listEdit', function() {
return {
    add: function(list, item) {
        list.push(item);
        return list;
    },
    remove: function(list, index) {
        list.splice(index, 1);
        return list;
    },
    count: function(list) {
        return list.length;
    }
}
});

//Error handling methods

toDo.controller('main', ['$scope', 'listEdit', function($scope, listEdit) {
$scope.list = [];

$scope.addToList = function() {
    list_length = listEdit.count($scope.list);
    if(list_length  1) {
    $scope.list = listEdit.remove($scope.list, index);
    } else {
        alert("You can't delete this last item");
    }
}

}]);

HTML

    
    Add to List
    
    {{todo}}
    

This is a funny question because

• One of the most readable Angular submissions on CR

• There is probably too much logic in the controller indeed

Some observations:

• It seems the controller knows too much, it should not know that 5 is the maximum amount of items, or that 1 is the minimum amount of items to be kept

• It seems silly to pass the list to list every single time to listEdit, it is not DRY.

• I know alert in this case is used because it is a prototype, but still..

I would counter propose something where listEdit is aware of the list after you make a connection, listEdit also knows about limits and gives feedback ( in the form of a string ) whenever it does something to the list.

The controller this way is very small, and easy to understand:

//Controller methods
toDo.controller('main', ['$scope', 'listEdit', function($scope, listEdit) {

  $scope.list = [];
  listEdit.linkList( $scope.list ); //<- Magic happens here

  $scope.addToList = function() {
    $scope.feedback = listEdit.add($scope.todo);
  };

  $scope.remove = function(index) {
    $scope.feedback =  listEdit.remove(index);
  };

}]);

listEdit is then a little bigger:

toDo.factory('listEdit', function() {
  var _list = [],
      maxLength = 5,
      minLength = 1;

  return {
    add: function(item) {
      if( _list.length >= maxLength )
        return 'Too many list items';
      _list.push(item);
      return item + ' is added';
    },
    remove: function(index) {
      if( _list.length == minLength )
        return 'You can\'t delete this last item';                
      var item = _list.splice(index, 1)[0];
      return item + ' has been deleted';
    },
    count: function() {
        return internalList.length;
    },
    linkList: function(list){
      _list = list;
    }
  };
});

I consciously chose for immediate return with feedback in case of a problem, then the intended activity and then giving positive feedback. My evil twin whispered this hackish alternative:

add: function(item) {
      return _list.length >= maxLength ?
        'Too many list items' :
        _list.push(item), item + ' is added';
    }

Also, you will find me railing against people using underscore to indicate that it is private when it is not. In this case, _list is actually private, so that is fine.

In order to show the feedback you would need to add div:

  
  JS Bin

    {{feedback}} 
    
    Add to List
    
    {{todo}}
    

I used the   trick to make sure the elements dont reflow after the first element is added, probably not best practice, HTML is not my forte.

In the end, it was fun to play with this in jsbin: http://jsbin.com/vesomu/2/edit

Also, check this : a simple todo app by Angular pro's.

StackExchange Code Review Q#51596, answer score: 8