Codecov Report

Merging #37 into master will increase coverage by <.01%. The diff coverage is 4.1%.

@@             Coverage Diff              @@
##             master      #37      +/-   ##
============================================
+ Coverage     38.18%   38.18%   +<.01%     
+ Complexity     9504     9502       -2     
============================================
  Files          1639     1639              
  Lines         83944    83926      -18     
  Branches      15321    15312       -9     
============================================
- Hits          32055    32050       -5     
+ Misses        48948    48935      -13     
  Partials       2941     2941

Impacted Files	Coverage Δ	Complexity Δ
...n/java/de/lmu/ifi/dbs/elki/math/IntegerMinMax.java	`80% <ø> (ø)`	`15 <0> (ø)`	:arrow_down:
...external/FileBasedSparseFloatDistanceFunction.java	`32.75% <0%> (+1.09%)`	`7 <0> (ø)`	:arrow_down:
...efunction/minkowski/ManhattanDistanceFunction.java	`53.12% <0%> (ø)`	`20 <0> (ø)`	:arrow_down:
...bs/elki/persistent/AbstractExternalizablePage.java	`47.05% <0%> (ø)`	`4 <0> (ø)`	:arrow_down:
...ncefunction/minkowski/MaximumDistanceFunction.java	`36.36% <0%> (ø)`	`15 <0> (ø)`	:arrow_down:
.../distancefunction/set/HammingDistanceFunction.java	`0% <0%> (ø)`	`0 <0> (ø)`	:arrow_down:
...ction/subspace/SubspaceLPNormDistanceFunction.java	`10.52% <0%> (ø)`	`4 <0> (ø)`	:arrow_down:
...ction/subspace/OnedimensionalDistanceFunction.java	`12.5% <0%> (ø)`	`3 <0> (ø)`	:arrow_down:
...mu/ifi/dbs/elki/utilities/pairs/DoubleObjPair.java	`35.71% <0%> (ø)`	`2 <0> (ø)`	:arrow_down:
...stogram/HistogramIntersectionDistanceFunction.java	`85.71% <0%> (ø)`	`7 <0> (ø)`	:arrow_down:
... and 60 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8ca0786...2b46e16. Read the comment docs.

kno10 commented 7 years ago

Two issues here:

You removed the obj == this || <full equals check> optimization in several places.
obj instanceof HardcodedClassName is not the same as this.getClass() != obj.getClass() because of subclassing. Thus, if someone subclasses one of these classes, the equals() methods with your modifications will produce wrong results. In your pull request, consider
```
LPNormDistanceFunction a = new LPNormDistanceFunction(3);
WeightedLPNormDistanceFunction b=new WeightedLPNormDistanceFunction(3,new double[]{2});
```
In your code a.equals(b) (but not b.equals(a))? Because b instanceOf LPNormDistanceFunction. But a.getClass() != b.getClass(), and they are not equal.

There is a good reason why Eclipse generates equals methods using getClass - this can lead to some really subtle and hard to find bugs (but yes, there are also cases where the stricter same-class requirement can be undesired). In my opinion, instanceOf in equals is only safe in final classes.

commandini commented 7 years ago

@kno10 Thanks for the explanation. :+1:

elki-project / elki

Simplified CASHInterval#compareTo() method. Changed code to favoring usage of 'instanceof'. #37

Codecov Report