Hi Tim,

Thanks for the response.

> The proposed naming is consistent with JavaScript’s Temporal
> (https://developer.mozilla.org/de/docs/Web/JavaScript/Reference/Global_Objects/Temporal/Instant)
> and Java’s java.time
> (https://docs.oracle.com/javase/8/docs/api/java/time/Instant.html),
> which are the primary sources of reference we used.
>
> I also believe that “real world time” and instead of “monotonic” time
> will be the primary use case and would thus prefer the simpler (and more
> widely used) name for that.

I think the JavaScript and Java background explains why I found the
naming confusing. `Instant` may feel natural for wall-clock time to
developers familiar with Java, and perhaps JavaScript, although
`Temporal` is still relatively new. Because i work more with Rust,
where `Instant` specifically represents monotonic time and
`SystemTime` represents wall-clock time.

I agree that wall-clock time will be used more often, but I’m not
convinced that `Instant` is necessarily simpler or clearer than
`SystemTime`. Arguably, the latter communicates more directly that the
value comes from the system clock and is subject to clock adjustments.
That said, let’s see what Derick and others think about the naming.

> For the real world clock, I think I would want to nudge users towards
> “doing the right thing” and use an injectable factory and therefore now
> provide the static factory, but I don't have particularly strong
> opinions regarding that. I don't think it should be a static method on
> the `Instant` class, though, because that would intimately tie it to the
> `SystemClock`, which feels wrong. The correct approach would probably be
> a `Time\now(): Instant` function.

Regarding nudging users towards “doing the right thing,” I don’t think
injecting a clock is always the right choice. It is valuable when the
code needs deterministic control over time, but sometimes keeping
things simple is preferable. For an occasional time read, passing a
clock through several layers can add ceremony without providing much
practical benefit. I think a direct API and injectable clocks can
coexist without suggesting that either should be used universally.

A `Time\now()` function would address the convenience issue, although
I still think discoverability is better when the operation is
available directly on the relevant type. But I don't have a strong
opinion here.

> > Also, If we are adding `now` static method, I think adding
> > [`elapsed()`](https://doc.rust-lang.org/src/std/time.rs.html#660-662)
> > would also be helpful. ( a shorter way of doing
> > `InstantOrSystemTime::now()->durationSince($this)` ).
>
> Similarly, I can see that being a first class citizen for the monotonic
> timer (since the prime use case is measuring elapsed time), but not so
> much for the real world clock, where measuring elapsed time is just one
> operation of many.

I also see some value in `elapsed()` for wall-clock instants, even
though I agree that it is more useful and reliable with a monotonic
clock. For example, I might use something like this for quick
debugging or logging:

```php
$startedAt = Instant::now();

$result = evaluate_expression($expr);

fwrite(STDERR, 'Took ' . $startedAt->elapsed());
```

For this kind of informal measurement, I may not need the stronger
guarantees of a monotonic clock. The operation would, of course,
remain subject to wall-clock adjustments. Rust returns an error if the
clock moves backwards; PHP could throw an exception in that case. I’m
not strongly attached to including this method for wall-clock time,
but I do think it has plausible uses.

> > - `FrozenClock` takes a time value and always returns that value. (
> > potentionally with `advance(Duration $duration): void` and
> > `travelTo(SystemTime $moment): void` )
> > - `OffsetClock` takes another clock and a duration, and returns the
> > wrapped clock’s current time plus that duration.
>
> I would be open to doing those two as an (immediate) follow-up, since I
> feel they have plenty of bike-shedding potential on their own. So they
> would also land in PHP 8.7, but not distract from the main API design.

I’m fine with leaving `FrozenClock` and `OffsetClock` to an immediate
follow-up RFC. My only concern is the current wording that test clocks
are better left to userland because their behaviour varies widely.
That argument could later be used against adding even these
well-defined clocks to the standard library. Perhaps they could
instead be mentioned explicitly under “Future Scope,” without
committing this RFC to their exact APIs.

Best regards,
Seifeddine

Reply via email to