vkostyukov / la4j

Linear Algebra for Java
http://la4j.org
Apache License 2.0
373 stars 109 forks source link

Equals() for Vector and Matrix #214

Closed SamoylovMD closed 9 years ago

SamoylovMD commented 9 years ago

Why don't we have equals() methods in Vector and Matrix interface?

Related: I see only one reason why we need MockVector and MockMatrix test classes - equals() method. So, I think that'll be good idea to move equals from mocks to AbstractVector and AbstractMatrix.

SamoylovMD commented 9 years ago

So, I understood the situation: mock classes were needed to avoid precision errors. Maybe we could rewrite equals code in AbstractVector to pass all tests without mocks? Current implementation is:

for (int i = 0; result && i < length; i++) {
    double a = get(i);
    double b = vector.get(i);

    double diff = Math.abs(a - b);

    result = (a == b) || (diff < Vectors.EPS || diff / Math.max(Math.abs(a), Math.abs(b)) < Vectors.EPS);
}
SamoylovMD commented 9 years ago

Bad idea: algorithms lower precision extremely. Maybe we could set custom precision when we compare vectors?

vkostyukov commented 9 years ago

Well, everything depends on Vectors.EPS constant. I agree, this is a good idea to pass it to a equals method. We may have an additional method int the interface that takes eps param. So, we can delegate the equals(Vector other) to equals(other, Vectors.EPS).

SamoylovMD commented 9 years ago

Yes, but there's another problem: assertEquals(Object o1, Object o2) calls equals(Object o) method only, always and everywhere, so even if we write new equals(Object o, double precision), this won't help us.

vkostyukov commented 9 years ago

Yeah. Why can't we just use MockMatrix and MockVector?

SamoylovMD commented 9 years ago

Another question: why can't we use special mock tools, i.e. mockito? We just need to change one method, sounds like work for mockito.

vkostyukov commented 9 years ago

Hm. That's sounds like a good idea. Can you try to plug it into la4j?

SamoylovMD commented 9 years ago

Mockito and other mock tools don't allow to stub equals() method, so I implemented equals(object, precision) and moved tests from MockVector and MockMatrix engine to new equals method, now it works. Also I marked MockVector and MockMatrix as deprecated.

So, new equals requires some additional testing, but I think we can close this issue.

vkostyukov commented 9 years ago

We can just remove them.