1

I'm trying to calculate a total by multiplying 2 ng-models. The models are in a ng-repeat, where I have a own controller for the rows.

On reload, it does the console log, but when I update the ng-models, they don't console log anymore and just don't work.

The controller:

app.controller('RowCtrl', function ($scope) {
   $scope.unit_price = 0;
   $scope.quantity = 0;

   $scope.$watchCollection('[unit_price, quantity]', function(newValues) {
      console.log(parseInt(newValues));
   }, true);

   $scope.total = $scope.unit_price * $scope.quantity;
});

UPDATED with fiddle: http://jsfiddle.net/r9Gmn/

guidsen
  • 2,333
  • 6
  • 34
  • 55
  • I think you should probably be using `$watch` not `$watchCollection`. See this question for information on that: http://stackoverflow.com/questions/16729965/how-to-watch-multiple-variable-change-in-angular – drew_w Jun 13 '14 at 14:55
  • @drew_w have already tried that. I think it's because I'm in my own controller within the ng-repeat. But it does console log on reload tho, just not updating. – guidsen Jun 13 '14 at 14:58
  • The overall question doesn't exactly make sense to me. I would suggest creating a fiddle that demonstrates the issue. – drew_w Jun 13 '14 at 14:59

4 Answers4

3

Watch a function that calculates the total:

$scope.$watch(function() { 
  return unit_price * quantity;
}, function(newVal) {
  $scope.total = newVal;
});
Michael Kang
  • 52,003
  • 16
  • 103
  • 135
  • 1
    `$watchCollection` is quite suitable here as it is used for watching _expressions_, **not** an _array on the scope_. Your's is a viable alternative, but OP approach works fine. Just a sidenote: This approach might be OK in this particular case, because the calculation is not very expensive. In other cases this might be inappropriate. It is a better practice to watch the items and perform any calculations **only** if they have indeed changed, instead of calculating in every $digest cycle. – gkalpak Jun 13 '14 at 15:09
  • @ExpertSystem using `$watchCollection` in this case is the worst for performance because it needs to create a new array (memory allocation) on the fly for each digest loop (not cycle) and then to compare the two arrays for equality. That's the reason why angular 1.3 introduced the $watchGroup method. – Ilan Frumer Jun 13 '14 at 15:27
  • @IlanFrumer: Yet, creating an array is more performant that performing a complax calculation. I am aware of 1.3's `$watchGroup()`, but since 1.3 is still in beta, I believe that the best tradeoff between complexity and efficiency in 1.2+ is `$watchCollection()`. – gkalpak Jun 13 '14 at 16:57
  • @IlanFrumer: BTW, the array won't be created with every $digest loop, because the Parser will parse the expression once, cache the parsed expression and only evaluate it at each $digest loop. – gkalpak Jun 13 '14 at 17:05
  • @pixelbits: I don't know what you base this on. I don't see why `$watch` would be supperior to `$watchCollection` in this case. I did some tests and there didn't seem to be a difference in performance. – gkalpak Jun 13 '14 at 19:46
  • @ExpertSystem Although it only parses the expression once still It needs to create a new array for each digest loop. The parser has an arrayDeclaration method which is used with json array declaration (in expressions), this is the line where the new array gets created https://github.com/angular/angular.js/blob/v1.2.18/src/ng/parse.js#L774 Again, this is the reason why `$watchGroup` was introduced. – Ilan Frumer Jun 14 '14 at 21:50
  • @IlanFrumer: 1. We agreed that `$watchGroup()` woukd be the best option **if** it were implemented in the stable version (which at time of answering wasn't). 2. The line you link to is **not** specific to `$watchCollection()` - it would be the same for `$watch()` as long as the same expression was used. 3. According to my understanding, the `arrayDeclaration()` function will only be executed as part of the parsing (thus only **once**) and **not** on every $digest loop. Am I wrong ? – gkalpak Jun 15 '14 at 05:57
  • @ExpertSystem you fail to understand the problem. Although `arrayDeclaration()` only run once, still it returns a function (https://github.com/angular/angular.js/blob/v1.2.18/src/ng/parse.js#L773) which would run each and every $digest loop. this is a known problem and they fixed it with $watchGroup (which doesn't use arrayDeclaration at all). please debug it by yourself before you make any other assumptions. – Ilan Frumer Jun 15 '14 at 08:34
  • @IlanFrumer: I didn't make any "assumptions". Anyway, I see what you mean, so my point (3) was invalid. Still points (1) and (2) are relevant. This is getting tiresome, so let's agree that we have different points of view on this (although I don't think we essentially disagree) and leave it there. – gkalpak Jun 15 '14 at 09:10
1

I agree with @pixelbits answer.

Just to add that as of angular 1.3 there is a new scope method $watchGroup:

An example http://plnkr.co/edit/2DvSmxUo5l8jAfBrSylU?p=preview

Solution:

$scope.$watchGroup(['unit_price', 'quantity'], function(val) {
  $scope.total = val[0] * val[1];
});
Ilan Frumer
  • 32,059
  • 8
  • 70
  • 84
0

This should work fine (if implemented correctly), i.e. your logic is correct:

<div ng-controller="myCtrl">
    Unit price:
    <input type="number" ng-model="unit_price" />
    <br />
    Quantity:
    <input type="number" ng-model="quantity" />
    <hr />
    Total: {{total}}
</div>

app.controller('myCtrl', function ($scope) {
    $scope.unit_price = 0;
    $scope.quantity   = 0;

    $scope.$watchCollection('[unit_price, quantity]', function(newValues) {
        $scope.total = $scope.unit_price * $scope.quantity;
    });
});

See, also, this short demo.

gkalpak
  • 47,844
  • 8
  • 105
  • 118
  • @guidsen: There were a few implementation issues with your fiddle, but the logic was correct. Here is the **[updated fiddle](http://jsfiddle.net/ExpertSystem/r9Gmn/2/)**. – gkalpak Jun 13 '14 at 17:14
  • `$scope.$watch('[unit_price, quantity]', ..., true)` would indeed work just as good (with the 3rd argument set to true). But I don't see why it would work better. – gkalpak Jun 13 '14 at 19:48
0

Here's your fiddle working: http://jsfiddle.net/mikeeconroy/r9Gmn/1/

In your $scope.rows array on the controller you never defined the properties to be used in the RowCtrl's scope. Also you should make sure you use track by with ng-repeat so you don't get the dupes error.

var app = angular.module('myApp', []);
app.controller('RowCtrl', function ($scope) {
    $scope.total = 0;
    $scope.$watchCollection('[row.unit_price, row.quantity]', function(newValues)         {
        $scope.total = parseInt(newValues[0]) * parseInt(newValues[1]);
    });
});

app.controller('MainCtrl', function ($scope) {
    $scope.rows = [
        { unit_price: 10, quantity: 0 },
        { unit_price: 12, quantity: 0 },
        { unit_price: 15, quantity: 0 },
    ];
});
m.e.conroy
  • 3,508
  • 25
  • 27