numbers / numbers.js

Advanced Mathematics Library for Node.js and JavaScript
Apache License 2.0
1.76k stars 167 forks source link

fix for #122 jshint warnings #125

Closed Dakkers closed 10 years ago

Dakkers commented 10 years ago

fix for #122. only two issues remain:

numbers.js/lib/numbers/basic.js
  line 124  col 23  Did you mean to return a conditional instead of an assignment?

   1 warning

numbers.js/lib/numbers/matrix.js
  line 51  col 36  Expected a conditional expression and instead saw an assignment.

   1 warning

everything else is taken care of.

cc @LarryBattle

LarryBattle commented 10 years ago

Looks good. The only thing I found strange was how in ./test/random.test.js, the variables test1, mu, sigma are being shared by all the test cases instead of being local variables.

Here's the fix for the last two.

File: numbers.js/lib/numbers/basic.js Line: 124 Old Code:

return arr[n][k] = _binomial(n - 1, k - 1) + _binomial(n - 1, k);

New Code:

arr[n][k] = _binomial(n - 1, k - 1) + _binomial(n - 1, k);
return arr[n][k];

numbers.js/lib/numbers/matrix.js Line: 51 Old Code:

matrix.isSquare = function(arr) {
    var rows = arr.length;

    for (var i = 0, row; row = arr[i]; i++) {
    if (row.length != rows) return false;
    }

    return true;
};

New Code:

matrix.isSquare = function(arr) {
    var rows = arr.length;

    for (var i = 0; i < rows; i++) {
    if (arr[i].length != rows) return false;
    }

    return true;
};

Code for if type checking is need

matrix.isSquare = function (arr) {
  if (!Array.isArray(arr)) {
    return false;
  }
  var len = arr.length;
  var isArray = Array.isArray;
  for (var i = 0, row; i < len; i++) {
    row = arr[i];
    if (!isArray(row) || row.length != len) {
      return false;
    }
  }

  return true;
};
Dakkers commented 10 years ago

@LarryBattle yeah, the shared variable thing was a quick fix. I'll go back and fix that today.

also, I should go through the functions to see what ones need type-checking. we're probably missing quite a few.

Dakkers commented 10 years ago

@LarryBattle alright it all seems good to go now. I'll merge it.