1

Background

I'm attempting to check the existence of a value in array A in a second array, B. Each value is an observable number. Each observable number is contained in an observable array. The comparison always returns -1, which is known to be incorrect (insofar as values in A and B overlap). Therefore, there's something wrong with my logic or syntax, but I have not been able to figure out where.

JSBin (full project): http://jsbin.com/fehoq/190/edit

JS

//set up my two arrays that will be compared
this.scores = ko.observableArray();

//lowest is given values from another method that splices from scores
this.lowest = ko.observableArray();

//computes and returns mean of array less values in lowest
this.mean = (function(scores,i) {
    var m = 0;
    var count = 0;

    ko.utils.arrayForEach(_this.scores(), function(score) {

        if (!isNaN(parseFloat(score()))) {

            //check values
            console.log(score());

            // always returns -1
            console.log(_this.lowest.indexOf(score()));

            //this returns an error, 'not a function'
            console.log(_this.lowest()[i]());

            //this returns undefined
            console.log(_this.lowest()[i]);

            //only do math if score() isn't in lowest
            // again, always returns -1, so not a good check
            if (_this.lowest.indexOf(score())<0) {
                m += parseFloat(score());
                count += 1;
            }
        }

    });

    // rest of the math
    if (count) {
        m = m / count;
        return m.toFixed(2);
    } else {
        return 'N/A';
    }
});

Update

@Major Byte noted that mean() is calculated before anything gets pushed to lowest, hence why I get undefined. If this is true, then what might be the best way to ensure that mean() will update based on changes to lowest?

Jefftopia
  • 2,105
  • 1
  • 26
  • 45
  • using the variable `i` is always going to fail if you don't give it a value as far as I can tell... – PW Kad May 09 '14 at 19:43
  • `i` is handed to it from a data-bind. It's `$index()` in a `foreach`. – Jefftopia May 09 '14 at 19:50
  • From what I can tell, student.mean is executed before student.lowest() is filled / initialized, which (student.lowest()) in turn only seems to be set/filled when you press the Drop Lowest Score button. Or am I looking at the wrong thing here? – Major Byte May 09 '14 at 19:58
  • Do you know what the error is at least at that point? – PW Kad May 09 '14 at 20:01
  • @PWKad, if that question was directed at me, then no, because there's no error thrown. But what Jefftopia wrote in the question here is for instance `// always returns -1 console.log(_this.lowest.indexOf(score()));` which is true, because at that moment the `_this.lowest` observable array is empty, because it only gets set/filled when the `Drop Lowest Score` button is pressed – Major Byte May 09 '14 at 20:06
  • Good point. Lowest is only filled when I click the button, which intuitively is desirable given the context, namely, that this is for a gradebook. So, my understanding is that by default, any function that relies on observables behaves like a computed...so, in this case, why isn't mean updating? I'm clearly missing something. – Jefftopia May 09 '14 at 20:13
  • I think you might be right, @MajorByte; the console log I commented prints undefined when written as `console.log(_this.lowest()[i]);`. Maybe it's undefined because it has no values? – Jefftopia May 09 '14 at 20:19
  • @Jefftopia That question was directed at you, but if it is undefined that doesn't mean it is an error only that no value existed at that index in the array. – PW Kad May 09 '14 at 20:23
  • True, but as I mentioned in my post, I already know that there are overlapping values. I used the chrome knockout debugger plugin and looked at each array's contents...hence why I can't tell why the check fails. – Jefftopia May 09 '14 at 20:26
  • @PWKad, the error for my original statement, `console.log(_this.lowest()[i]());`, was `not a function`. Updating the original post to clarify the two similar, but different log statements, and what they return. – Jefftopia May 09 '14 at 20:36
  • Regarding your added question from the latest edit, "what might be the best way to ensure that mean() will update based on changes to lowest" you should probably make `mean` a `ko.computed`. Since the evaluation of the `mean` computed will be reading from both the `scores` and the `lowest` observables, the `mean` computed will be reevaluated every time either of those observables change. I suggest you read up on [the documentation on Knockout computed observables](http://knockoutjs.com/documentation/computedObservables.html). – Robert Westerlund May 09 '14 at 21:09
  • A `ko.computed` does not take arguments, so I don't think that will work. This question leads me to think that a regular function that accepts observables as arguments should be sufficient. http://stackoverflow.com/questions/23299194/knockout-js-indexof-always-returns-1 – Jefftopia May 09 '14 at 21:12
  • Why would you want to have arguments of the mean is based on this.scores and this.lowest? – Major Byte May 09 '14 at 21:14
  • Mean needs to update when changes are made to `scores` and `lowest`. – Jefftopia May 09 '14 at 21:20

1 Answers1

3

You really just could use a computed for the mean

this.mean = ko.computed(
    function() {
      var sum  = 0;
      var count = 0;
      var n = 0;
      for(n;n < _this.scores().length;n++)
      {
        var score = _this.scores()[n];
          if (_this.lowest.indexOf(score)<0) {
            sum += parseFloat(score());
            count++;
          }
      }

      if (count > 0) {
        sum = sum / count;
        return sum.toFixed(2);
      } else {
        return 'N/A';
      }
});

this will trigger when you add to lower(), scores() and change scores().

obligatory jsfiddle.

Update:
Forgot to mention that I change something crucial as well. From you original code:

this.dropLowestScores = function() {
    ko.utils.arrayForEach(_this.students(), function(student){
        var comparator = function(a,b){
            if(a()<b()){
                return 1;
            } else if(a() > b()){
                return -1;
            } else {
                return 0;
            }
        };
        var tmp = student.scores().sort(comparator).slice(0);
        student.lowest = ko.observableArray(tmp.splice((tmp.length-2),tmp.length-1));
    });
 };

apart from moving the comparator outside of dropLowestScores function, I changed the line:

student.lowest = ko.observableArray(tmp.splice((tmp.length-2),tmp.length-1));

to

student.lowest(tmp.splice((tmp.length-2),tmp.length-1));

student.lowest is an observable array, no need to define it as an observableArray again, in fact that actually breaks the computed mean. (The correction for Drop Lowest Scores as per my previous comment is left out here).

Major Byte
  • 4,101
  • 3
  • 26
  • 33
  • The `Drop Lowest Score` currently drops all scores except the highest though. Will see I can fix that as well. – Major Byte May 09 '14 at 21:48
  • 1
    The `Drop Lowest Score` is working correctly now as well – Major Byte May 09 '14 at 22:02
  • A while ago I tried to do this with a computed, stack told me not to. I see that this works perfectly well, but now I'm even more unsure when one should vs. shouldn't use a `ko.computed`. – Jefftopia May 10 '14 at 15:26
  • @Jefftopia, could you point me to the question/answer you are refering to? – Major Byte May 11 '14 at 07:43
  • Here, http://stackoverflow.com/questions/23299194/knockout-js-indexof-always-returns-1. So I'm hung up on the bindings internally doing to computed part, versus just writing a method that is a `ko.computed`. – Jefftopia May 11 '14 at 21:30