On Wed, Apr 14, 2010 at 3:28 PM, Jake Mannix <jake.man...@gmail.com> wrote: > What is the transitivity problem? If (a instanceof VClassA), (b instanceof > VClassB) and (c instanceof VClassC), if all three equals() methods compare > the same things (ie values, names, not implementation), then a.equals(b) && > b.equals(c) ==> a.equals(c), right?
The potential issue comes in two flavors. Say the current vector implementations equals() without regard to implementation, which is by no means wrong per se. But then say someone implements MyVector which is only equals() to MyVectors -- which is conventional for equals() implementations. This breaks transitivity since a DenseVector might equals() a MyVector but not vice versa. .. and I now see that's really a symmetry problem not transitivity. But oh well. That's just a coding error but a subtle one. The other possibility is implementing a, I dunno, ColoredVector which adds colors to each dimension. Even if ColoredVector implements equals() correctly and without regard to implementation class, transitivity breaks when two ColoredVectors with different colors but identical values are compared to a standard Vector implementation. They're both equals() to it but not equals() each other. > I would actually prefer ripping names out of the base vectors entirely. > They should decorate the mathematical vector, but as their use is decidedly > non-mathematical and application specific (we basically don't use them at > all currently, it should be noted, and remember the YAGNI principle...), > keeping them in the base mahout-math vector classes seems wrong. You read my mind. I agree entirely and it simplifies things here a lot. I'd also say this about labels, but first things first perhaps. > If they weren't in the base class at all, we could have just > > == : ref check > equals : check values, not impl (I support not checking implementation, though we have to be cognizant of the issues above. For example a NamedVector implementation later, or any extension, would have to not consider a name as part of the state for purposes of equality. Which could be a fine restriction for this sort of class.) If I can get any other +1s I'll prepare a patch.
