Skip to content

Commit f765cf5

Browse files
committed
chore: LongRange Does Not Support Empty Range
* Removed the special case for `-1 -> -1` LongRange * We used to have this to denote "empty" range * We've since moved on from this, because this is extra complexity with no added benefit * Fixed unit tests that asserted the possibility of `-1 -> -1` LongRange NOTE: This change also comes with the need to validate that we do not disrupt or break current logic, which _could_ be using the empty range notion! LongRange has almost 400 usages at the time of this commit. Thorough walkthrough is needed to validate that nothing is broken. Signed-off-by: Atanas Atanasov <a.v.atanasov98@gmail.com>
1 parent e7eb4c3 commit f765cf5

File tree

2 files changed

+44
-56
lines changed
  • block-node/spi-plugins/src

2 files changed

+44
-56
lines changed

block-node/spi-plugins/src/main/java/org/hiero/block/node/spi/historicalblocks/LongRange.java

Lines changed: 43 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -7,59 +7,41 @@
77

88
/**
99
* Contiguous range of long values, inclusive of start and end.
10-
* Valid ranges must have start and end values between 0 and Long.MAX_VALUE-1 inclusive,
11-
* with start less than or equal to end. This ensures that the size() method can correctly
12-
* represent any valid range with a long value without risk of overflow.
10+
* Valid ranges must have start and end values between 0 and Long.MAX_VALUE-1
11+
* inclusive, with start less than or equal to end. This ensures that the size()
12+
* method can correctly represent any valid range with a long value without risk
13+
* of overflow.
1314
*/
1415
public record LongRange(long start, long end) implements Comparable<LongRange> {
15-
1616
/** Comparator for comparing LongRange objects by their start and end values. */
1717
public static final Comparator<LongRange> COMPARATOR =
1818
Comparator.comparingLong(LongRange::start).thenComparingLong(LongRange::end);
1919

2020
/**
2121
* Creates a new LongRange with the specified start and end values.
2222
*
23-
* @param start the start value of the range (inclusive), must be between 0 and Long.MAX_VALUE-1
24-
* @param end the end value of the range (inclusive), must be between 0 and Long.MAX_VALUE-1
25-
* @throws IllegalArgumentException if start or end is negative or greater than Long.MAX_VALUE-1,
26-
* or if start is greater than end
23+
* @param start the start value of the range (inclusive), must be between 0
24+
* and Long.MAX_VALUE-1
25+
* @param end the end value of the range (inclusive), must be between 0 and
26+
* Long.MAX_VALUE-1
27+
* @throws IllegalArgumentException if start or end is negative or greater
28+
* than Long.MAX_VALUE-1, or if start is greater than end
2729
*/
2830
public LongRange {
29-
// Special case: allow both start and end to be -1 for clean state initialization
30-
if (!(start == -1 && end == -1)) {
31-
if (start < 0) {
32-
throw new IllegalArgumentException("Range start must be non-negative: " + start);
33-
}
34-
if (end < 0) {
35-
throw new IllegalArgumentException("Range end must be non-negative: " + end);
36-
}
37-
if (start > end) {
38-
throw new IllegalArgumentException(
39-
"Range start must be less than or equal to end: " + start + " > " + end);
40-
}
41-
if (end > Long.MAX_VALUE - 1) {
42-
throw new IllegalArgumentException("Range end must be less than or equal to Long.MAX_VALUE-1: " + end);
43-
}
31+
if (start < 0) {
32+
throw new IllegalArgumentException("LongRange start: %d must not be negative".formatted(start));
33+
}
34+
if (end < 0) {
35+
throw new IllegalArgumentException("LongRange end: %d must not be negative".formatted(end));
36+
}
37+
if (end > Long.MAX_VALUE - 1) {
38+
throw new IllegalArgumentException(
39+
"LongRange end: %d must not be greater than Long.MAX_VALUE-1".formatted(end));
40+
}
41+
if (start > end) {
42+
throw new IllegalArgumentException(
43+
"LongRange start: %d must not be greater than end: %d".formatted(start, end));
4444
}
45-
}
46-
47-
/**
48-
* Gets the start value of the range, inclusive.
49-
*
50-
* @return the start value of the range
51-
*/
52-
public long start() {
53-
return start;
54-
}
55-
56-
/**
57-
* Gets the end value of the range, inclusive.
58-
*
59-
* @return the end value of the range
60-
*/
61-
public long end() {
62-
return end;
6345
}
6446

6547
/**
@@ -68,25 +50,34 @@ public long end() {
6850
* @param value the value to check
6951
* @return true if the range contains the value, false otherwise
7052
*/
71-
public boolean contains(long value) {
53+
public boolean contains(final long value) {
7254
return value >= start && value <= end;
7355
}
7456

7557
/**
76-
* Checks if the range contains another range specified by start and end values.
58+
* Checks if the range contains another range specified by start and end
59+
* values.
7760
*
7861
* @param start the start value of the range to check
7962
* @param end the end value of the range to check
8063
* @return true if the range contains the specified range, false otherwise
8164
*/
82-
public boolean contains(long start, long end) {
83-
return start >= this.start && end <= this.end;
65+
public boolean contains(final long start, final long end) {
66+
if (start > end) {
67+
// if the caller supplies invalid values, responsibility falls on
68+
// them, we must return a response that makes sense based on input,
69+
// in this case false, as no range can start after it ends
70+
return false;
71+
} else {
72+
return start >= this.start && end <= this.end;
73+
}
8474
}
8575

8676
/**
8777
* Gets the size of the range.
8878
*
89-
* @return the size of the range (number of elements), computed as end - start + 1
79+
* @return the size of the range (number of elements),
80+
* computed as end - start + 1
9081
*/
9182
public long size() {
9283
return end - start + 1;
@@ -98,7 +89,7 @@ public long size() {
9889
* @param other the other range to check
9990
* @return true if the ranges overlap, false otherwise
10091
*/
101-
public boolean overlaps(LongRange other) {
92+
public boolean overlaps(final LongRange other) {
10293
return !(end < other.start() || start > other.end());
10394
}
10495

@@ -118,7 +109,7 @@ public boolean isAdjacent(LongRange other) {
118109
* @param other the other range to merge with
119110
* @return a new ImmutableLongRange representing the merged range
120111
*/
121-
public LongRange merge(LongRange other) {
112+
public LongRange merge(final LongRange other) {
122113
return new LongRange(Math.min(start, other.start()), Math.max(end, other.end()));
123114
}
124115

@@ -135,11 +126,11 @@ public LongStream stream() {
135126
* Compares this range to another range.
136127
*
137128
* @param o the other range to compare to
138-
* @return a negative integer, zero, or a positive integer as this range is less than, equal to, or greater than
139-
* the specified range
129+
* @return a negative integer, zero, or a positive integer as this range is
130+
* less than, equal to, or greater than the specified range
140131
*/
141132
@Override
142-
public int compareTo(@NonNull LongRange o) {
133+
public int compareTo(@NonNull final LongRange o) {
143134
return COMPARATOR.compare(this, o);
144135
}
145136

@@ -148,6 +139,7 @@ public int compareTo(@NonNull LongRange o) {
148139
*
149140
* @return a string representation of the range in the format "start->end"
150141
*/
142+
@NonNull
151143
@Override
152144
public String toString() {
153145
return start + "->" + end;

block-node/spi-plugins/src/test/java/org/hiero/block/node/spi/historicalblocks/LongRangeTest.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,8 @@ void testConstructor() {
3737
assertEquals(10, range3.start());
3838
assertEquals(10, range3.end());
3939

40-
// Test special case for clean state initialization
41-
final LongRange cleanStateRange = new LongRange(-1, -1);
42-
assertEquals(-1, cleanStateRange.start());
43-
assertEquals(-1, cleanStateRange.end());
44-
4540
// Test validation failures
41+
assertThrows(IllegalArgumentException.class, () -> new LongRange(-1, -1));
4642
assertThrows(IllegalArgumentException.class, () -> new LongRange(-1, 5));
4743
assertThrows(IllegalArgumentException.class, () -> new LongRange(5, -1));
4844
assertThrows(IllegalArgumentException.class, () -> new LongRange(6, 5));

0 commit comments

Comments
 (0)