MINOR: Minor updates to RangeSet (#19678)

Minor updates to RangeSet:   * Disallow ranges with negative size   *
Disallow ranges with more than Integer.MAX_VALUE elements   * Fix
equals() so that all empty RangeSets are equal, to follow the Set
interface definition better.   * Reimplement hashCode() to follow the
Set interface definition.

Reviewers: Ken Huang <s7133700@gmail.com>, PoAn Yang
<payang@apache.org>, Chia-Ping Tsai <chia7712@gmail.com>
This commit is contained in:
Sean Quah 2025-05-30 19:31:51 +01:00 committed by GitHub
parent 5e601b2b26
commit 8323168b57
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 62 additions and 7 deletions

View File

@ -40,6 +40,14 @@ class RangeSet implements Set<Integer> {
public RangeSet(int from, int to) { public RangeSet(int from, int to) {
this.from = from; this.from = from;
this.to = to; this.to = to;
if (to < from) {
throw new IllegalArgumentException("Invalid range: to must be greater than or equal to from");
}
if ((long) to - (long) from > Integer.MAX_VALUE) {
throw new IllegalArgumentException("Range exceeds the maximum size of Integer.MAX_VALUE");
}
} }
@Override @Override
@ -163,6 +171,7 @@ class RangeSet implements Set<Integer> {
if (!(o instanceof Set<?> otherSet)) return false; if (!(o instanceof Set<?> otherSet)) return false;
if (o instanceof RangeSet other) { if (o instanceof RangeSet other) {
if (this.size() == 0 && other.size() == 0) return true;
return this.from == other.from && this.to == other.to; return this.from == other.from && this.to == other.to;
} }
@ -176,8 +185,16 @@ class RangeSet implements Set<Integer> {
@Override @Override
public int hashCode() { public int hashCode() {
int result = from; // The hash code of a Set is defined as the sum of the hash codes of its elements.
result = 31 * result + to; // The hash code of an integer is the integer itself.
return result;
// The sum of the integers from 1 to n is n * (n + 1) / 2.
// To get the sum of the integers from 1 + k to n + k, we can add n * k.
// So our hash code comes out to n * (from + to - 1) / 2.
// The arithmetic has to be done using longs, since the division by 2 is equivalent to
// shifting the 33rd bit right.
long sum = size() * ((long) from + (long) to - 1) / 2;
return (int) sum;
} }
} }

View File

@ -24,6 +24,7 @@ import java.util.NoSuchElementException;
import java.util.Set; import java.util.Set;
import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals;
@ -32,6 +33,19 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
public class RangeSetTest { public class RangeSetTest {
@Test
void testNegativeSize() {
assertThrows(IllegalArgumentException.class, () -> new RangeSet(0, -1));
}
@Test
void testOverflow() {
// This range is the maximum size we allow.
assertDoesNotThrow(() -> new RangeSet(0, Integer.MAX_VALUE));
assertThrows(IllegalArgumentException.class, () -> new RangeSet(-1, Integer.MAX_VALUE));
}
@Test @Test
void testSize() { void testSize() {
RangeSet rangeSet = new RangeSet(5, 10); RangeSet rangeSet = new RangeSet(5, 10);
@ -117,15 +131,39 @@ public class RangeSetTest {
assertEquals(rangeSet1, set); assertEquals(rangeSet1, set);
assertEquals(rangeSet3, hashSet); assertEquals(rangeSet3, hashSet);
assertNotEquals(rangeSet1, new Object()); assertNotEquals(rangeSet1, new Object());
// Empty sets are equal.
RangeSet emptyRangeSet1 = new RangeSet(0, 0);
RangeSet emptyRangeSet2 = new RangeSet(5, 5);
Set<Integer> emptySet = Set.of();
assertEquals(emptySet, emptyRangeSet1);
assertEquals(emptySet, emptyRangeSet2);
assertEquals(emptyRangeSet1, emptyRangeSet2);
} }
@Test @Test
void testHashCode() { void testHashCode() {
RangeSet rangeSet1 = new RangeSet(5, 10); RangeSet rangeSet1 = new RangeSet(5, 10);
RangeSet rangeSet2 = new RangeSet(5, 10); RangeSet rangeSet2 = new RangeSet(6, 10);
RangeSet rangeSet3 = new RangeSet(6, 10);
assertEquals(rangeSet1.hashCode(), rangeSet2.hashCode()); assertEquals(Set.of(5, 6, 7, 8, 9).hashCode(), rangeSet1.hashCode());
assertNotEquals(rangeSet1.hashCode(), rangeSet3.hashCode()); assertEquals(Set.of(6, 7, 8, 9).hashCode(), rangeSet2.hashCode());
RangeSet emptySet = new RangeSet(5, 5);
assertEquals(Set.of().hashCode(), emptySet.hashCode());
// Both these cases overflow the range of an int when calculating the hash code. They're
// chosen so that their hash codes are almost the same except for the most significant bit.
RangeSet overflowRangeSet1 = new RangeSet(0x3FFFFFFD, 0x3FFFFFFF);
RangeSet overflowRangeSet2 = new RangeSet(0x7FFFFFFD, 0x7FFFFFFF);
assertEquals(
Set.of(0x3FFFFFFD, 0x3FFFFFFE).hashCode(),
overflowRangeSet1.hashCode() // == 0x0_FFFFFFF6 / 2
);
assertEquals(
Set.of(0x7FFFFFFD, 0x7FFFFFFE).hashCode(),
overflowRangeSet2.hashCode() // == 0x1_FFFFFFF6 / 2
);
} }
} }