Hi Andrej,
Yes that looks like my first implementation. But then
I reflected that avoiding to map Optional to Duration
and then back to a new Optional containing the same
duration could be avoided by simply storing the original
optional obtained from the HttpClient.
The current code only creates a new instance of Optional
when it needs to.
best regards,
-- daniel
On 18/01/2019 12:42, Andrej Golovnin wrote:
Hi Daniel,
webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8216561/webrev.01/
126 private static class ConnectTimeoutTracker {
127 final Optional<Duration> max;
128 final AtomicLong startTime = new AtomicLong();
129 ConnectTimeoutTracker(Optional<Duration> connectTimeout) {
130 this.max = connectTimeout;
131 }
132
133 Optional<Duration> getRemaining() {
134 if (max.isEmpty()) return max;
135 long now = System.nanoTime();
136 long previous = startTime.compareAndExchange(0, now);
137 if (previous == 0 || max.get().isZero()) return max;
138 Duration remaining =
max.get().minus(Duration.ofNanos(now - previous));
139 assert remaining.compareTo(max.get()) <= 0;
140 return Optional.of(remaining.isNegative() ?
Duration.ZERO : remaining);
141 }
142
143 void reset() { startTime.set(0); }
144 }
An Optional in a class field or in a data structure, is considered a
misuse of the API (see the explanation by Stuart Marks:
https://stackoverflow.com/questions/23454952/uses-for-optional/23464794#23464794).
I would write it this way:
private static class ConnectTimeoutTracker {
final Duration max;
final AtomicLong startTime = new AtomicLong();
ConnectTimeoutTracker(Duration connectTimeout) {
this.max = connectTimeout;
}
Optional<Duration> getRemaining() {
return Optional.ofNullable(max)
.map(m -> {
long now = System.nanoTime();
long previous = startTime.compareAndExchange(0, now);
if (previous == 0 || m.isZero()) {
return m;
}
Duration remaining = m.minus(Duration.ofNanos(now
- previous));
assert remaining.compareTo(m) <= 0;
return remaining.isNegative() ? Duration.ZERO :
remaining;
});
}
void reset() { startTime.set(0); }
}
Best regards,
Andrej Golovnin
