Final Comment Period for the Duration API

[llogiq]

millis and nanos abbreviating milliseconds and nanoseconds, respectively, has a long-standing tradition. Together with the seconds getter, their meaning should be clear enough even when abbreviated.

On the other hand, the extra_nanos name is not entirely clear on its own. nanos_part might be more clear, IMHO. Or even subsecond_nanos, but that’s a bit long.

[DanielKeep]

The problem is that nanos_part implies that it’s only nanoseconds; not milli- or microseconds. Actually, that’s a problem with secs/seconds: I’ve seen APIs that provide each “tier” independently, and it wouldn’t be obvious from a glance what this API is doing.

What about as_secs for “this duration represented as seconds”? I think that has a much stronger implication that it’s the whole duration, not just the seconds portion.

In that case, I’d also go for subsec_nanos over extra_nanos or nanos_part. That said, I don’t think extra_nanos is much worse, but for one extra character, subsec_nanos seems clearer.

It’d also be very nice to have convenience methods that deal in f64s (e.g. as_frac_secs) for when you want sub-second precision, but don’t care about nanosecond precision. Obviously, those can be bolted on later, though they’d be nice to have in the first pass none the less.

[llogiq]

Agreed, subsec_nanos makes it even more clear.

I don’t quite like as_secs, because it implies a cast of the whole value, while I see the function rather as an accessor (after all, it is one).

[DanielKeep]

Only because you’re aware that it’s storing itself in two components: seconds and nanoseconds.

Imagine that you’re used to working with something like std::tm; you see “secs” and immediately think “oh, it’s the seconds component. How do I get the whole thing?”

And anyway, there is precedent for this. Consider std::slice::as_ptr. That is also technically an accessor, but is logically a cheap transform of the value from a slice to a pointer. Same thing here: it does a cheap transform of a Duration into an integer number of seconds.

Also, it kind-of is a cast of the whole value… a truncating one, but still.

[llogiq]

I agree that the distinction is a bit blurry. But naming the accessor as_secs actively keeps users in the dark, while seconds doesn’t (though I’ll grant that the hint of being an accessor is quite subtle), with the same amount of characters and IMHO better readability.

Also, if you wanted to keep method naming consistent, you’d also need to name the other accessor/truncating cast as_subsec_nanos, which just confuses me.

[DanielKeep]

But it’s not logically an accessor or truncating cast. It’s computing a property of the value. You’re only confused because you know how it’s actually implemented. It shouldn’t matter to the user whether Duration has a u64/u32 split, is a u128, or a bignum.

Also, my point was that as_secs is not and should not be seen as an accessor. Again, as far as a user is concerned, they should only know that a Duration is some quantity of time. How it’s stored is irrelevant.

Unless the plan is to make the internal representation part of the interface, in which case, go ahead.

[llogiq]

So if it’s not a truncating cast, why should we name is as_…? Naming it seconds only tells the user that it is no cast and not consuming the value. Btw. perhaps my confusion stems from my Java backgrounds, where accessors (X getThis(), void setThat(X x) ) have been in widespread use expressly because they allow to hide how the result is computed or what action is performed. As a user of the API, I don’t care about the underlying computation, only that duration.seconds() always returns the value in seconds, without mutating the underlying structure in any visible way.

[DanielKeep]

No, no; subsec_nanos isn’t a truncating cast, as_secs is a truncating cast. I probably should have quoted the part of your response, but I’m lazy.

Again, my primary reason for wanting as_secs is to distance it from interfaces where something called seconds would return only the seconds part. In the grand scheme of things, seconds/as_secs is a relatively minor point.

[llogiq]

Ah, I see. Thank you for clearing up my confusion.

[kstep]

I’m as a user a little confused there’s a from_millis constructor but there’s no millis getter. Yes, yes, I understand why it’s implemented this way, I just want to say this kind of symmetry between setting and getting values is expected and missing it feels a little strange to me.

[jpfed]

Again, my primary reason for wanting as_secs is to distance it from interfaces where something called seconds would return only the seconds part.

I’m sure this has been noted somewhere but I’m fond of the .NET TimeSpan approach of using Seconds as “the seconds part” and TotalSeconds as “this duration as a whole, expressed in seconds”.

[jminer]

As someone who has used .NET’s TimeSpan, I remember finding its naming clear. I quickly knew which method I wanted just by reading the method names.

[eridius]

What about as_secs for “this duration represented as seconds”? I think that has a much stronger implication that it’s the whole duration, not just the seconds portion.

In that case, I’d also go for subsec_nanos over extra_nanos or nanos_part. That said, I don’t think extra_nanos is much worse, but for one extra character, subsec_nanos seems clearer.

I agree with this. I don’t like sec() or seconds() because it’s not clear whether it’s trying to represent the whole value as seconds, or just the “seconds part” of the value. as_secs() clears this right up, that makes it clear it’s representing the entire value as seconds (although obviously it loses subsecond precision).

I also like subsec_nanos, as it’s a bit more clear to someone who doesn’t know the internal structure of Duration than extra_nanos.

I’m also tempted to say we should have as_millis, as_micros, and as_nanos, but they’d have to return Option<u64> values (or else panic on overflow), which makes them not as compelling.

[dhardy]

So let me get this straight:

• A Duration is a high-precision measure of some moment in time relative to an earlier Epoch.
• A Period is presumably like the one in Joda-Time, and makes no sense without a calendar implementation
• An Instant is a combination of a Duration and an Epoch

Aside from the calendar, this is missing the Epoch and negative durations. Will the Epoch be fixed, or some template parameter of the Instant type?

In one of my projects I have found negative durations to be useful, but in a context where a custom time type was required anyway.

Perhaps more relevant to current question: what are Mul and Div for? There should possibly be multiple versions:

Duration * f64 -> Duration    // scale a duration
Duration / f64 -> Duration    // scale a duration
Duration / Duration -> f64    // calculate a ratio

Duration * Duration makes no sense unless you want units of time squared, which cannot be correctly stored in a Duration. I also see no point implementing multiplication and division against integer types… come to that I’m not sure why you’d want multiplication or division in a context where you weren’t going to convert the all inputs to floating point anyway, except as a shortcut to say things like “test 1 took 3.2 times longer to run than test 2”.

[arielb1]

Instant only needs to have an epoch for its representation - it doesn’t have a semantic epoch. Duration tries to be an SI-second duration, with no relevant epoch.

[kryptan]

I don’t see much justification for not allowing negative durations. RFC says that non-negative duration is the predominant use-case but I’m not sure about this. I think that panicking in Sub if result is negative can break otherwise valid algorithms.

[retep998]

I agree that allowing negative durations would be quite useful when you’re doing a lot of time related math.

[wycats]

I don’t see much justification for not allowing negative durations

The problem with negative Duration is that every function that takes a Duration now has to decide what to do with negative ones.

[sfackler]

Thanks for the feedback, everyone! I’ve updated the PR to rename secs to as_secs and extra_nanos to subsec_nanos.

[system]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.