18

I'm using Ionic and want to dynamically change the background colour of each item in an <ion-list> based on the data. I thought I'd do this by way of a function call to return the correct class

<ion-list>
  <ion-item ng-repeat="singleCase in allCases" ng-class="getBackgroundColour(singleCase)" class="item-avatar">
    <h2>{{singleCase.date}}</h2>
    <p>{{singleCase.caseType}}</p>
  </ion-item>
</ion-list>

This is the controller at present

  .controller('AllCasesCtrl', ['$scope', '$log', 'dummyData', function($scope, $log, dummyData) {
    $scope.allCases = dummyData.cases;

    $scope.getBackgroundColour = function(singleCase){

      $log.log("getBackgroundColour called...singleCase type: " + singleCase.speciality);

      var colourMap = {
        speciality1: "speciality1Class",
        speciality2: "speciality2Class",
        speciality3: "speciality3Class"
      };

      return colourMap[singleCase.speciality];
    };

  }])

On checking the console, the getBackgroundColour() function is being called 7 times for each <ion-item> in the list. Why is this, and should I avoid using a function call in ng-class?

cortexlock
  • 1,446
  • 2
  • 13
  • 26

2 Answers2

27

AngularJS works with dirty checking: it needs to call the function each cycle to be sure that it doesn't return a new value and that the DOM doesn't need to be updated.

It's part of the typical process of the framework, and calling a function as simple as yours shouldn't have any negative performance impact. Readability and testability of your code is far more important here, so keep the logic in your controller.

One simple things to do, however, is simply to move the declaration of colourMap, which is a constant, outside of your function:

var colourMap = {
    speciality1: "speciality1Class",
    speciality2: "speciality2Class",
    speciality3: "speciality3Class",
};

$scope.getBackgroundColour = function(singleCase) {
  return colourMap[singleCase.speciality];
};
Blackhole
  • 20,129
  • 7
  • 70
  • 68
  • Let's say I have 100 generated elements, it will call my function 100*100 = 10 000 times. Is there another solution to bypass the dirty checking system ? As a newbie with AngularJS, I saw the inheritence between controllers can be used to store data but I don't know if it's a "best practice". Can someone bring me light on this ? thx – Alex May 27 '15 at 15:15
  • 2
    @Alex Where does the second "100" in your calculus come from? You can evaluate your expressions only once if you know they won't change (that's the [lazy one-time binding](http://stackoverflow.com/q/23969926/2057033)), but you can't "bypass" the dirty checking system. How do you know your values have changed without it? – Blackhole May 27 '15 at 20:02
  • yeah I've written this a bit too quickly. I wanted to say that the function is called (100 + 99 + 98 + 97 ... + 2 + 1) times (with my 100 elements example). By bypassing the dirty checking system, it's here my bad english which betrayed me. I wanted to ask if there were a way to check the value only once, by using another approach. And finally, thanks so much for your lazy one-time binding, i'm going to look trough this. – Alex Jun 01 '15 at 08:01
  • @Alex I don't understand: if you've 100 elements, the function is only called 100 times, unless you add the new items in the corresponding array one after the other, with at least a digest cycle between. – Blackhole Jun 01 '15 at 13:22
  • Apparently my first formula was the right one : I'll explain you why I'm saying this. I've got an array of 3 objects in my angular controller. From the HTML template, I loop through this array with ng-repeat. alongside the ng-repeat instruction, I've added a ng-class one, with a function in it. like `getCssClass({{ element.type }})`. Into this function, I've put a console.log, which is displayed twice per element. So I've got 6 console outputs, insteand of the 3 I should have. Is my explanation clear enough ? (sorry again about my english) – Alex Jun 01 '15 at 13:38
  • @Alex Yes, so that's basically one time for each element (and for each digest cycle), so 100 times for 100 elements. Your English is perfect, by the way ;) . – Blackhole Jun 01 '15 at 13:54
8

Your way is fine as long as your list is not some huge size. That being said if you are using angular 1.3 and you want to lower the number of calls you can change your ng-class to ng-class="::getBackgroundColour(singleCase)". This applies one time binding so once the value is stable it will not check again. This would most likely mean two calls per item.

Jason
  • 1,316
  • 13
  • 25
  • Thanks, will have to go and read about this. The classes should remain very stable so I could try this. But if the user goes and changes the data, the class would need to updated...would using the one-time binding break this? – cortexlock Nov 18 '14 at 11:19
  • as long as you don't use one-time binding for allCases variable then it will call it again since the ng-repeat will execute and create new dom elements – Jason Nov 18 '14 at 18:20