https://issues.apache.org/bugzilla/show_bug.cgi?id=46962
--- Comment #21 from Alex Giotis <alex.gio...@gmail.com> 2012-03-22 02:02:02 UTC --- Vincent, thanks for looking at it. > * Why declare the equals and hashCode methods on interfaces (Numeric, > PercentBase)? They are defined on Object anyway, and AFAICT declaring them on > interfaces wouldn't change anything (that is, force the developer to implement > them on sub-classes?). The definitions of hashcode/equals on the interfaces are not needed. It was helping me easily locate which classes implement them. Can be removed. > * Why define those methods as abstract on Property? What if a sub-class is > perfectly happy with the default implementations from Object? Also, it doesn't > allow to call super.hashCode or super.equals any more. I'm not sure at all if > this is desirable. This was for forcing future sub-classes to implement them as it is not obvious when an implementation is needed. There are properties like ListProperty (a list of Property instances) and classes like the CompoundPropertyMaker which use it. For example, the FontFamilyProperty extends ListProperty and uses the cache. If we rely on the implementations from Object, then the caching is simply an overhead as the FontFamilyProperty#make method will never create two properties with the same identity (Object#equals uses the == operator). Having said that, if this is not desirable, the abstract methods on Property can be removed. > * Why use Double.doubleToLongBits in equals methods to compare doubles > (NumericProperty, PercentLength)? Why not just thisDouble == otherDouble? (One > could also argue that some epsilon might be necessary, but this is another > topic.) Well, if we forget about the epsilon, the doubleToLongBits is the correct way to check for equality two doubles. This is well explained in Effective Java by Joshua Bloch, used in Double#compare, Double#equals and by all IDEs that generate them and libraries like Apache commons-lang EqualsBuilder. Java has double values for both 0.0 and -0.0, as well as the never equal "not a number" (NaN). Those can't be checked with ==. Have a look into the doubleToLongBits source and at the javadoc for Double.equals(). > * shouldn't the specVal field from Property also be tested for equality? It seems that currently this is not needed. The specVal is set by 3 properties, the FontShorthandProperty, the LineHeightProperty and the URIProperty. None of them uses the cache, so currently it is not needed in the equality tests. Generally, it should be safe to demand from every class using the cache to correctly implement the hashCode/equal methods. Unfortunately, we can not in general enforce it. With that view, it could be better to remove the abstract declarations on Property. Please tell me if you expect an updated patch. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug.
