Improve vector by uniformizing the interface #12
Conversation
|
This pull request treats 2D vectors as 3D vectors with a z-component of 0. Depending on the client this could break backwards compatibility. |
| @@ -46,14 +46,14 @@ Matrix.prototype = { | |||
| }, | |||
|
|
|||
| dot: function (B) { | |||
| if (B.isVector ? B.v.length !== this.cols : B.rows !== this.cols) { | |||
| if (B.isVector ? B.v.length < this.cols : B.rows !== this.cols) { | |||
| return new Error('number of cols of A must equal number of rows of B'); | |||
There was a problem hiding this comment.
Why change !== to <? Isn't it also an issue if B.v.length > this.cols?
There was a problem hiding this comment.
This is done to allow a 2x2 matrix operate on a 2D vector. Because this pull request treats 2D vectors as 3D vectors with a z-component 0, therefor B.v.length is 3.
So the 'simplest' solution to that problem was to allow matrices of any size act upon vectors just as long as there is a vector-element for each matrix-row, i.e. B.v.length >= this.cols.
Having said that, I agree that it is not intuitive. Because this is a consequence of treating 2D vectors as disguised 3D vectors, you can wonder if that is a good choice to do. There are alternatives.
|
@rockbot Do not forget that this pull request is a suggestion, and that an possible response is: "I do not like it, for these reasons". |
|
@rockbot I was wondering if you would like to see a different implementation or that you are ok with the one provided in this pull-request? A different implementation would for example treat 2D vectors as vectors with 2 components and have a 'lift' method to lift them into a 3D vector. What would you prefer? |
addresses issue #11