[jira] [Resolved] (MATH-1689) Frequency.equals()/hashCode() ignores the comparator

["Alex Herbert (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/MATH-1689?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Alex Herbert resolved MATH-1689.
--------------------------------
    Fix Version/s: 4.0-beta2
       Resolution: Fixed

Documented in commit:
 
148f3d43fda95123b7a2a3f80cce64dd7bbdd174
 

> Frequency.equals()/hashCode() ignores the comparator
> ----------------------------------------------------
>
>                 Key: MATH-1689
>                 URL: https://issues.apache.org/jira/browse/MATH-1689
>             Project: Commons Math
>          Issue Type: Improvement
>          Components: legacy
>            Reporter: Ruiqi Dong
>            Priority: Minor
>             Fix For: 4.0-beta2
>
>
> *Summary*
> Frequency behavior depends on the comparator used by its internal map, but equals() and hashCode() ignore that comparator. Two Frequency objects with different ordering semantics can therefore compare equal.
> *Affected code*
> File: commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/Frequency.java
> {code:java}
> public long getCumFreq(T v) {
>     ...
>     Comparator<? super T> c = freqTable.comparator();
>     if (c == null) {
>         c = new NaturalComparator<>();
>     }
>     ...
> }
> public List<T> getMode() {
>     ...
> }
> @Override
> public int hashCode() {
>     final int prime = 31;
>     int result = 1;
>     result = prime * result + ((freqTable == null) ? 0 : freqTable.hashCode());
>     return result;
> }
> @Override
> public boolean equals(Object obj) {
>     ...
>     Frequency<?> other = (Frequency<?>) obj;
>     return freqTable.equals(other.freqTable);
> } {code}
>  
> *Reproducer*
> Add the following test to commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/stat/FrequencyTest.java:
> {code:java}
> @Test
> public void testEqualsShouldConsiderComparator() {
>     Frequency<Integer> ascending = new Frequency<>();
>     Frequency<Integer> descending = new Frequency<Integer>(Comparator.reverseOrder());
>     ascending.addValue(1);
>     ascending.addValue(2);
>     descending.addValue(1);
>     descending.addValue(2);
>     Assert.assertEquals(1, ascending.getCumFreq(1));
>     Assert.assertEquals(2, descending.getCumFreq(1));
>     Assert.assertEquals("[1, 2]", ascending.getMode().toString());
>     Assert.assertEquals("[2, 1]", descending.getMode().toString());
>     Assert.assertNotEquals(ascending, descending);
> } {code}
> Run:
> {code:java}
> mvn -q -pl commons-math-legacy -Dtest=org.apache.commons.math4.legacy.stat.FrequencyTest test {code}
> Observed behavior:
> {code:java}
> FrequencyTest.testEqualsShouldConsiderComparator:223
> Values should be different. Actual:
> Value 	 Freq. 	 Pct. 	 Cum Pct.
> 2	1	50%	50%
> 1	1	50%	100% {code}
> Expected behavior:
> Frequencies with different comparators should not compare equal when their observable ordering behavior differs.
>  
> The comparator affects at least getCumFreq() and getMode(). Since those outputs differ, equality and hashing should not ignore the comparator.
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)