Fix an edge case in Matrix#isIdentity.

[hynek-urban]

This pull request should resolve issue #3.

[koggdal]

Good catch! And thanks for also fixing it 👍

I seem to have thought about this in the constructor where identity data only is set if it is a square matrix. But calling setIdentityData still sets data even if it wouldn't actually be identity data in the case of non-square matrices. What do you think about having setIdentityData fall back to calling setEmptyData internally for non-square matrices? And then the constructor wouldn't need to have that check. It's not strictly correct either though, since it's not really valid to set identity data for a non-square matrix, but maybe OK. Another option would be to throw an error when called on non-square matrices.

[hynek-urban]

Hi,

thanks for the reply. I did not read the entire source code as I was just fixing a specific problem that I encountered - thus I didn't notice that the problem applies to setIdentityData as well.

From the two options you mention, throwing an error seems more correct to me. In my eyes, a third option would be even more natural, namely - not having a public method "setIdentityData" on Matrix prototype at all :) But breaking backward compatibility wouldn't be worth it in this case.

Hynek

[hynek-urban]

Although, thinking of it, throwing an Error for non-square matrices in setIdentityData breaks backward compatibility anyway :)

[koggdal]

I would agree actually, not having the method at all would make sense for a general purpose matrix class. Let's remove it! I'm not that scared of breaking changes. Upgrading to a new major version of a package is easier when it's small changes :)