Expose Time#total_seconds in public API

[straight-shoota]

Time#total_seconds was a protected method, but it can be useful outside of Time namespace, for example to implement custom extensions to the Time API.
This method directly exposes an implementation detail, but it doesn't depend on the implementation and would be useful even if the implementation was changed. It supplements the already existing constructor Time.new(seconds, nanoseconds) which accepts total_seconds since epoch as argument.

I don't think Time#offset_seconds needs to be exposed since it can easily be duplicated as sum of #total_seconds and #offset. Instead I changed visibility to private because it doesn't need to be exposed in the namespace (and isn't even used).

See also #5374 (probably fixes that).

[ysbaddaden]

Unless you have clear use cases, this method is private API that reflect internal implementation details of Time and Time::Span.

Time.new(sec, nsec) should be marked with nodoc for the same reason —I believe it used to be. Maybe some other methods should be, too.

[ysbaddaden]

Actually, I believe the #total_seconds and #offset_seconds don't even have to exist. Just use @seconds and @seconds + @offset where needed. They're only used within Time itself and seem more confusing than really helpful.

[straight-shoota]

Use case: custom extensions to the Time API need a direct way to extract essential information out of a Time instance.

If #total_seconds and the supplementing constructor didn't exist, this would only be possible through the unix timestamp or the calendrical interface. Both are not optimal and require additional calculations which might be completely useless.
I don't see any benefit from hiding these methods.

Yes, it directly exposes the internal representation, but that isn't bad by itself. seconds since epoch is a common property of any time representation, and it would make sense to have this method even if the internal implementation was different (for example using fields).

I agree that #offset_seconds is not really helpful.

[RX14]

Who says we don't change the epoch later? And then because this is public API, #total_seconds won't be useful for anything. I guess since the community decided foo.@var was a good feature you can just use that.

[straight-shoota]

Who says we don't change the epoch later?

It's the epoch of the ISO 8601 calendar. We can change the internal representation (including epoch) however we like, but the calendar epoch won't change unless we change the calendar.
And in that case, there are more breaking changes than #total_seconds.

@seconds is exactly the thing we'd like to avoid: having to rely on internal implementation.

With the current implementation, #total_seconds just returns @seconds. But imagine the case where we'd change the internal implementation, for example to use Unix epoch instead. Then #total_seconds would just change to @seconds - UNIX_SECONDS and the public API doesn't break.

[ysbaddaden]

I don't see any benefit from hiding these methods.

You don't expose any use case for them to be exposed either. Time extensions that would run slower isn't a use case. It's an hypothetical case.

@seconds is exactly the thing we'd like to avoid: having to rely on internal implementation.

No, this is exactly what we want: this is an internal value, don't use it, or use at your own risk, because it will break at the most unexpected time. We don't even need Time#total_seconds in stdlib, only Time uses it, and it thus should use @seconds. We already had the issue with #ticks which completely broke when we changed the internal representation, and didn't care to fix. Let's not reiterate.

[straight-shoota]

Time extensions that would run slower isn't a use case. It's an hypothetical case.

They don't exist yet. But there are enough features missing from the stdlib implementation (and shouldn't be needed there) that sooner or later there will be third party extensions. You can call it hypothetical, but that's a very realistic use case.

Yes, Time can use @seconds directly. But an external library shouldn't have to. #total_seconds doesn't expose implementation details, it just happens that the implementation uses that same property "seconds since epoch" as internal representation.

Let's take a look at #epoch: Its value didn't change when the internal representation changed from ticks to seconds/nanoseconds. And it won't change if it changes back to ticks or something different. The same is true for #total_seconds - and similarly, the constructor new(seconds, nanoseconds).

Described in different words: #total_seconds is like #epoch (or #unix_seconds when #6662 is merged) just with the native epoch of the ISO calendar (0001-01-01) instead of Unix epoch (1970-01-01). And .new(seconds, nanoseconds) is like .epoch (or .unix) but with calendar epoch and nanosecond resolution.

The only argument against #total_seconds is that there is already a very similar #epoch/#unix_seconds and we might not need both.
That's certainly up for debate.

I still think it makes sense to have #total_seconds because together with new(seconds, nanoseconds) it provides a smooth and consistent interface based on the inherent design of the underlying calendar and allows to convert a Time instance to a primitive datatype and back without losing precision (unix methods don't support nanoseconds).

[asterite]

I'm not sure about this. Without a use case it's hard to approve this PR. I think converting a Time from/to unix seconds is good enough. I can't see why the numbers of seconds since 01-01-01 is important (given that we have a "standard" measure against the unix epoch).

[straight-shoota]

If we add nanosecond precision to the UNIX constructor it might be fine.

[straight-shoota]

Closing stale PR.