Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Duration equals and compareTo when values differ by very little #33

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix Duration equals and compareTo when values differ by very little #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e762bdf
Test Duration.compareTo
findepi Jul 3, 2023
0b93efc
Fix Duration.compareTo for sub-millis differences
findepi Jul 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 157 additions & 25 deletions src/main/java/io/airlift/units/Duration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.regex.Pattern;

import static io.airlift.units.Preconditions.checkArgument;
import static java.lang.Double.isFinite;
import static java.lang.Math.floor;
import static java.lang.String.format;
import static java.util.Objects.requireNonNull;
Expand Down Expand Up @@ -102,7 +103,150 @@ public TimeUnit getUnit()
public double getValue(TimeUnit timeUnit)
{
requireNonNull(timeUnit, "timeUnit is null");
return value * (millisPerTimeUnit(this.unit) * 1.0 / millisPerTimeUnit(timeUnit));
switch (unit) {
case DAYS:
switch (timeUnit) {
case DAYS:
return value;
case HOURS:
return value * 24;
case MINUTES:
return value * (24 * 60);
case SECONDS:
return value * (24 * 60 * 60);
case MILLISECONDS:
return value * (24 * 60 * 60 * 1000);
case MICROSECONDS:
return value * (24 * 60 * 60 * 1000 * 1000L);
case NANOSECONDS:
return value * (24 * 60 * 60 * 1000 * 1000L * 1000);
default:
throw new IllegalArgumentException("Unsupported time unit " + timeUnit);
}

case HOURS:
switch (timeUnit) {
case DAYS:
return value / 24;
case HOURS:
return value;
case MINUTES:
return value * 60;
case SECONDS:
return value * (60 * 60);
case MILLISECONDS:
return value * (60 * 60 * 1000);
case MICROSECONDS:
return value * (60 * 60 * 1000 * 1000L);
case NANOSECONDS:
return value * (60 * 60 * 1000 * 1000L * 1000);
default:
throw new IllegalArgumentException("Unsupported time unit " + timeUnit);
}

case MINUTES:
switch (timeUnit) {
case DAYS:
return value / (24 * 60);
case HOURS:
return value / 60;
case MINUTES:
return value;
case SECONDS:
return value * 60;
case MILLISECONDS:
return value * (60 * 1000);
case MICROSECONDS:
return value * (60 * 1000 * 1000);
case NANOSECONDS:
return value * (60 * 1000 * 1000 * 1000L);
default:
throw new IllegalArgumentException("Unsupported time unit " + timeUnit);
}

case SECONDS:
switch (timeUnit) {
case DAYS:
return value / (24 * 60 * 60);
case HOURS:
return value / (60 * 60);
case MINUTES:
return value / 60;
case SECONDS:
return value;
case MILLISECONDS:
return value * (1000);
case MICROSECONDS:
return value * (1000 * 1000);
case NANOSECONDS:
return value * (1000 * 1000 * 1000);
default:
throw new IllegalArgumentException("Unsupported time unit " + timeUnit);
}

case MILLISECONDS:
switch (timeUnit) {
case DAYS:
return value / (24 * 60 * 60 * 1000);
case HOURS:
return value / (60 * 60 * 1000);
case MINUTES:
return value / (60 * 1000);
case SECONDS:
return value / 1000;
case MILLISECONDS:
return value;
case MICROSECONDS:
return value * 1000;
case NANOSECONDS:
return value * (1000 * 1000);
default:
throw new IllegalArgumentException("Unsupported time unit " + timeUnit);
}

case MICROSECONDS:
switch (timeUnit) {
case DAYS:
return value / (24 * 60 * 60 * 1000 * 1000L);
case HOURS:
return value / (60 * 60 * 1000 * 1000L);
case MINUTES:
return value / (60 * 1000 * 1000);
case SECONDS:
return value / (1000 * 1000);
case MILLISECONDS:
return value / 1000;
case MICROSECONDS:
return value;
case NANOSECONDS:
return value * 1000;
default:
throw new IllegalArgumentException("Unsupported time unit " + timeUnit);
}

case NANOSECONDS:
switch (timeUnit) {
case DAYS:
return value / (24 * 60 * 60 * 1000 * 1000L * 1000);
case HOURS:
return value / (60 * 60 * 1000 * 1000L * 1000);
case MINUTES:
return value / (60 * 1000 * 1000 * 1000L);
case SECONDS:
return value / (1000 * 1000 * 1000);
case MILLISECONDS:
return value / (1000 * 1000);
case MICROSECONDS:
return value / 1000;
case NANOSECONDS:
return value;
default:
throw new IllegalArgumentException("Unsupported time unit " + timeUnit);
}

default:
throw new IllegalArgumentException("Unsupported time unit " + unit);
}
}

public long roundTo(TimeUnit timeUnit)
Expand Down Expand Up @@ -193,9 +337,19 @@ public static Duration valueOf(String duration)
}

@Override
public int compareTo(Duration o)
public int compareTo(Duration that)
{
return Double.compare(getValue(MILLISECONDS), o.getValue(MILLISECONDS));
double thisValueMillis = this.getValue(MILLISECONDS);
double thatValueMillis = that.getValue(MILLISECONDS);
int compare = Double.compare(thisValueMillis, thatValueMillis);
if (compare == 0) {
double thisValueNanos = this.getValue(NANOSECONDS);
double thatValueNanos = that.getValue(NANOSECONDS);
if (isFinite(thisValueNanos) && isFinite(thatValueNanos)) {
compare = Double.compare(thisValueNanos, thatValueNanos);
}
}
return compare;
}

public boolean isZero()
Expand Down Expand Up @@ -270,26 +424,4 @@ public static String timeUnitToString(TimeUnit timeUnit)
throw new IllegalArgumentException("Unsupported time unit " + timeUnit);
}
}

private static double millisPerTimeUnit(TimeUnit timeUnit)
{
switch (timeUnit) {
case NANOSECONDS:
return 1.0 / 1000000.0;
case MICROSECONDS:
return 1.0 / 1000.0;
case MILLISECONDS:
return 1;
case SECONDS:
return 1000;
case MINUTES:
return 1000 * 60;
case HOURS:
return 1000 * 60 * 60;
case DAYS:
return 1000 * 60 * 60 * 24;
default:
throw new IllegalArgumentException("Unsupported time unit " + timeUnit);
}
}
}
54 changes: 51 additions & 3 deletions src/test/java/io/airlift/units/TestDuration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,26 @@ public void testConstructorRejectsNullUnit()
new Duration(1, null);
}

@Test
public void testCompareTo()
{
assertComparison(new Duration(12359.0d, MILLISECONDS), new Duration(12359.0d, MILLISECONDS), 0);
assertComparison(new Duration(0, MILLISECONDS), new Duration(12359.0d, MILLISECONDS), -1);
assertComparison(new Duration(1, MILLISECONDS), new Duration(0, MILLISECONDS), 1);
assertComparison(new Duration(1, NANOSECONDS), new Duration(0, NANOSECONDS), 1);
assertComparison(new Duration(3, NANOSECONDS), new Duration(2, NANOSECONDS), 1);
assertComparison(new Duration(3, NANOSECONDS), new Duration(3, NANOSECONDS), 0);

assertComparison(new Duration(1, DAYS), new Duration(24, HOURS), 0);
assertComparison(new Duration(1, DAYS), new Duration(24 * 60, MINUTES), 0);
assertComparison(new Duration(1, DAYS), new Duration(24 * 60 * 60, SECONDS), 0);
assertComparison(new Duration(1, DAYS), new Duration(24 * 60 * 60 * 1000, MILLISECONDS), 0);
assertComparison(new Duration(1, DAYS), new Duration(24 * 60 * 60 * 1000 * 1000L, MICROSECONDS), 0);
assertComparison(new Duration(1, DAYS), new Duration(24 * 60 * 60 * 1000 * 1000L * 1000, NANOSECONDS), 0);

assertComparison(new Duration(0, NANOSECONDS), new Duration(Double.MIN_VALUE, NANOSECONDS), -1);
}

@Test
public void testEquals()
{
Expand All @@ -309,7 +329,7 @@ public void testIsZero()

// very small values
assertFalse(new Duration(Double.MIN_VALUE, MILLISECONDS).isZero());
assertTrue(new Duration(Double.MIN_VALUE, NANOSECONDS).isZero()); // TODO incorrect, due to equals converting to MILLIS
assertFalse(new Duration(Double.MIN_VALUE, NANOSECONDS).isZero());
}

@Test
Expand All @@ -327,7 +347,7 @@ public void testNanoConversions()
assertThat(duration.getValue()).isEqualTo(nanos);
assertThat(duration.getValue(NANOSECONDS)).isEqualTo(nanos);
assertThat(duration.getValue(MILLISECONDS)).isEqualTo(nanos / 1000000);
assertThat(duration.getValue(SECONDS)).isEqualTo(nanos / 1000000 / 1000);
assertThat(duration.getValue(SECONDS)).isEqualTo(nanos / (1000000 * 1000));
assertThat(duration.getValue(MINUTES)).isCloseTo(nanos / 1000000 / 1000 / 60, offset(1.0E10));
assertThat(duration.getValue(HOURS)).isCloseTo(nanos / 1000000 / 1000 / 60 / 60, offset(1.0E10));
assertThat(duration.getValue(DAYS)).isCloseTo(nanos / 1000000 / 1000 / 60 / 60 / 24, offset(1.0E10));
Expand Down Expand Up @@ -492,6 +512,34 @@ private Object[][] conversions()
new Object[] {DAYS, MINUTES, 60 * 24},
new Object[] {DAYS, HOURS, 24},
new Object[] {DAYS, DAYS, 1},
};
};
}

private static void assertComparison(Duration left, Duration right, int expectedSign)
{
if (expectedSign == 0) {
assertThat(left)
.isEqualTo(right)
.isEqualByComparingTo(right);
assertThat(right)
.isEqualTo(left)
.isEqualByComparingTo(left);
}
else if (expectedSign < 0) {
assertThat(left)
.isNotEqualTo(right)
.isLessThan(right);
assertThat(right)
.isNotEqualTo(left)
.isGreaterThan(left);
}
else {
assertThat(left)
.isNotEqualTo(right)
.isGreaterThan(right);
assertThat(right)
.isNotEqualTo(left)
.isLessThan(left);
}
}
}