ExecutionContext#getStartTime() returns Duration
brainbytes42 opened this issue · 6 comments
Is it intentionally to return Duration instead of Instant?
From the docs I'd assume the getElapsedTime() would be the Duration between the getStartTime()-"should-be-Instant" and now...? Or what's the difference otherwise?
--
footnote: just going through 3.x-changes, looks great so far! (was still on 2.x-version...) :-)
Thanks for bringing this up. getElapsedTime() works as you say, here's the implementation. But getStartTime is indeed problematic since it uses System.nanoTime internally, which returns nanoseconds since some arbitrary start time. You're right that returning an Instant here would be more appropriate.
There's a few deprecated methods in the API that I've been waiting to remove. I may just cut a 3.3 release to do that and make this change at the same time. Normally I'd prefer to deprecate an API before removing it, but in this case it's really a change, not a deprecation, that we want. An alternative would be for the returned Duration to be based on System.currTimeMillis, but returning a Duration here was just never a good idea.
nice, that was a quick fix! 👍
I am actually wondering if getStartTime should return Optional<Instant> since the start time will be null if an execution is cancelled before it begins (such as if it's waiting on a thread and a timeout occurs). Previously this method was never returning a null.
There are other methods that may return null, such as ExecutionCompletedEvent.getResult or getException, but I wonder if returning null from getStartTime might be more surprising.
That makes a lot of sense to me. It's easy to miss that something can be null, but it's impossible not to notice an Optional.
Closed again with ed8de67.
This has been released in 3.3.0.