HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Manipulating model elements in angular

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
elementsmodelangularmanipulating

Problem

All,
I barely have a week of angular behind me and I am still struggling with some of the concepts. Below is a piece of code I came up with. The idea is to let the user add/delete entries in a model (in practice, the model is read from a json and is more complex than what is presented). While this code seems to work, I'm wondering whether it's "proper" angular:

  • Is it OK to leave the add_item function in the controller ? I understand that any DOM manipulation should be put in a directive, but it's only manipulating the model, right?



  • Shouldn't the del_item be in the controller too, then?



  • Using the index to control which item to delete look a bit dangerous, what would be a better alternative?



Thanks for any input.


  
    
  

 

 
   {{items}}
   
     
       New item
     
   
   
     
       
     
   
 

  angular.module("main", [])
  .controller("BaseController",
    function($scope){
      $scope.items = [{key:1, a:'A'},{key:2, a:'B'},{key:3, a:'C'}];
      $scope.last = $scope.items.length;
      $scope.add_item = function() {
        var item={key: $scope.last + 1, a:'?'};
        $scope.items.push(item);
        $scope.last += 1;
        };
    }
   )
  .directive('displayItem',
    function() {
    return {
      restrict: 'E',
      transclude: true,
      scope: {idx: '@'},
      template: '{{idx}}:'+
                '{{$parent.items[idx].key}}={{$parent.items[idx].a}}'+
                'X',
      link: function(scope, element, attrs) {
        scope.del_item=function(i){
          scope.$parent.items.splice(i,1)}
        }
      }
    }
  );

Solution

-
Right, you're only manipulating the model, updating the model about a change (thus) updating it in the controller is perfectly fine, and it's what the controller does.

-
del_item should be in the controller too, you are correct. That's where it belongs.

-
Using the index to deleting the item is dangerous, what I'd do is pass this as a function and then use an indexOf to locate the element you're deleting and delete that.

If I may ask, why are your display items in a directive in the first place and not in the DOM? You could use an ng-repeat in the DOM to repeat the items instead and drop the directive altogether.

Directives are useful for code reuse, are you reusing this spefici component in a lot of other places through out your code? Behaviors affecting your models and not just your presentation logic should probably not be in the directive to begin with (like del_item in your example), a directive should encompass presentational behavior and not business logic.

Context

StackExchange Code Review Q#27506, answer score: 2

Revisions (0)

No revisions yet.