Skip to main content
added 12 characters in body
Source Link
Flambino
  • 33k
  • 2
  • 45
  • 90
  • You misspelled echelon in the function name ("echolon")

  • Conventionally, names in JavaScript are camelCased, not under_scored

  • You have a lot of really short variable names. Yes, there are comments explaining them where they're declared, but that doesn't help much, when you're in the middle of a dense line later, having to refer back to the comments again and again. Yes, lines will get longer with longer variable names, but they'll also become a lot more readable!

  • There's no need to first assign an empty object to a variable, and then add properties to that object. It'd be clearer to define the object's initial state in one go:

      len = {
        i: matrix.length,    // matrix length (rows)
        j: matrix[0].length  // vector length (columns)
      };
    

    Looking at the above code, the object should perhaps be called simply size, and i and j could well be called rows and columns - it's what the comments say, so you might as well just call them that.

  • You misspelled echelon in the function name

  • Conventionally, names in JavaScript are camelCased, not under_scored

  • You have a lot of really short variable names. Yes, there are comments explaining them where they're declared, but that doesn't help much, when you're in the middle of a dense line later, having to refer back to the comments again and again. Yes, lines will get longer with longer variable names, but they'll also become a lot more readable!

  • There's no need to first assign an empty object to a variable, and then add properties to that object. It'd be clearer to define the object's initial state in one go:

      len = {
        i: matrix.length,    // matrix length (rows)
        j: matrix[0].length  // vector length (columns)
      };
    

    Looking at the above code, the object should perhaps be called simply size, and i and j could well be called rows and columns - it's what the comments say, so you might as well just call them that.

  • You misspelled echelon in the function name ("echolon")

  • Conventionally, names in JavaScript are camelCased, not under_scored

  • You have a lot of really short variable names. Yes, there are comments explaining them where they're declared, but that doesn't help much, when you're in the middle of a dense line later, having to refer back to the comments again and again. Yes, lines will get longer with longer variable names, but they'll also become a lot more readable!

  • There's no need to first assign an empty object to a variable, and then add properties to that object. It'd be clearer to define the object's initial state in one go:

      len = {
        i: matrix.length,    // matrix length (rows)
        j: matrix[0].length  // vector length (columns)
      };
    

    Looking at the above code, the object should perhaps be called simply size, and i and j could well be called rows and columns - it's what the comments say, so you might as well just call them that.

added 49 characters in body
Source Link
Flambino
  • 33k
  • 2
  • 45
  • 90
  • You misspelled echelon in the function name

  • Conventionally, names in JavaScript are camelCased, not under_scored

  • You have a lot of really short variable names. Yes, there are comments explaining them where they're declared, but that doesn't help much, when you're in the middle of a dense line later, having to refer back to the comments again and again. Yes, lines will get longer with longer variable names, but they'll also become a lot more readable!

  • There's no need to first assign an empty object to a variable, and then add properties to that object. It'd be clearer to define the object's initial state in one go:

      len = {
        i: matrix.length,    // matrix length (rows)
        j: matrix[0].length  // vector length (columns)
      };
    

    Looking at the above code, the object should perhaps be called simply size, and i and j could well be called rows and columns - it's what the comments say, so you might as well just call them that.

  • Conventionally, names in JavaScript are camelCased, not under_scored

  • You have a lot of really short variable names. Yes, there are comments explaining them where they're declared, but that doesn't help much, when you're in the middle of a dense line later, having to refer back to the comments again and again. Yes, lines will get longer with longer variable names, but they'll also become a lot more readable!

  • There's no need to first assign an empty object to a variable, and then add properties to that object. It'd be clearer to define the object's initial state in one go:

      len = {
        i: matrix.length,    // matrix length (rows)
        j: matrix[0].length  // vector length (columns)
      };
    

    Looking at the above code, the object should perhaps be called simply size, and i and j could well be called rows and columns - it's what the comments say, so you might as well just call them that.

  • You misspelled echelon in the function name

  • Conventionally, names in JavaScript are camelCased, not under_scored

  • You have a lot of really short variable names. Yes, there are comments explaining them where they're declared, but that doesn't help much, when you're in the middle of a dense line later, having to refer back to the comments again and again. Yes, lines will get longer with longer variable names, but they'll also become a lot more readable!

  • There's no need to first assign an empty object to a variable, and then add properties to that object. It'd be clearer to define the object's initial state in one go:

      len = {
        i: matrix.length,    // matrix length (rows)
        j: matrix[0].length  // vector length (columns)
      };
    

    Looking at the above code, the object should perhaps be called simply size, and i and j could well be called rows and columns - it's what the comments say, so you might as well just call them that.

Source Link
Flambino
  • 33k
  • 2
  • 45
  • 90

Partial answer. Wait around and I'm sure someone will do the actual reduction function justice. It's a bit much for me to digest right now.

From a cursory glance, though, I'd suggest breaking things out into functions where possible. But again, I haven't delved too deep.

Overall stuff:

  • Conventionally, names in JavaScript are camelCased, not under_scored

  • You have a lot of really short variable names. Yes, there are comments explaining them where they're declared, but that doesn't help much, when you're in the middle of a dense line later, having to refer back to the comments again and again. Yes, lines will get longer with longer variable names, but they'll also become a lot more readable!

  • There's no need to first assign an empty object to a variable, and then add properties to that object. It'd be clearer to define the object's initial state in one go:

      len = {
        i: matrix.length,    // matrix length (rows)
        j: matrix[0].length  // vector length (columns)
      };
    

    Looking at the above code, the object should perhaps be called simply size, and i and j could well be called rows and columns - it's what the comments say, so you might as well just call them that.

The above things are fixable without changed to the actual logic. What's more troublesome to me is that your reduced_row_echelon_form function produces side effects: It alters the matrix you pass it, rather than returning a new one. After calling it, you have the answer, but you've lost the question.

I'd suggest making a lot more use of map(). Again, I haven't had the courage to dive into the reduced_row_echelon_form function, but - for instance - the sorting function can be cleaned up rather nicely using it:

function sortRows(matrix) {
  // map rows to arrays with the structure
  // [<column index>, <pivot coefficient>, <row index>]
  // which makes them easily sortable
  var pivots = matrix.map(function (row, r) {
    for(var c = 0, l = row.length ; c < l ; c++) {
      if(row[c]) {
        return [c, row[c], r]; // found a non-zero coefficient at matrix[r][c]
      }
    }
    return [Infinity, null, r]; // row is all-zero
  });

  // sort the pivots array, and use it to (re)build
  // the matrix with the rows in the correct order
  return pivots.sort().map(function (pivot) {
    return matrix[pivot[2]];
  });
}

Which will do something like (in some sort of ascii-notation)

[ 0, 0, 0 ]                   [ 1, 2, 3 ]
[ 0, 0, 0 ]                   [ 0, 1, 3 ]
[ 0, 2, 1 ]         =>        [ 0, 2, 1 ]
[ 0, 1, 3 ]                   [ 0, 0, 3 ]
[ 1, 2, 3 ]                   [ 0, 0, 0 ]
[ 0, 0, 3 ]                   [ 0, 0, 0 ]