Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

time: add Duration-conversion functions Milliseconds() and Microseconds() #28564

Closed
tiegz opened this issue Nov 2, 2018 · 6 comments
Closed
Labels
FeatureRequest FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Milestone

Comments

@tiegz
Copy link

tiegz commented Nov 2, 2018

I propose adding these two Duration-conversion functions to time.go:

func (d Duration) Milliseconds() float64
func (d Duration) Microseconds() float64

(these were already proposed in #5491 as int64 and rejected as WorkingAsIntended)

Here's a map of Duration-conversion helpers in time.go -- all added in efe3d35 -- missing the two functions above:

Unit Duration constants Duration conversion
Nanosecond time.Nanosecond func (d Duration) Nanoseconds() int64
Microsecond time.Microsecond
Millisecond time.Millisecond
Second time.Second func (d Duration) Seconds() float64
Minute time.Minute func (d Duration) Minutes() float64
Hour time.Hour func (d Duration) Hours() float64

Reasons

  • assymetry: as shown above, there's an asymmetry in the time API. Reasons I've seen for not implementing these is generally "just divide by Millisecond", but this results in application code that would be written with two different styles of conversion, e.g. time.Since(time.Now()).Hours() vs time.Since(time.Now()) / Millisecond
  • newcomer perspective: as a newcomer to Go this year, I found it helpful that Go has a duration type, but was confused why it doesn't have a Milliseconds() helper.
  • usefulness: milliseconds (and microseconds) are common units for measuring web response times, database response times, high-frequency trading, gaming, etc (e.g. Google research, High Scalability blog), justifying their inclusion.
  • demand: I've found some other accounts of people who want these functions: example, example, example
  • language parity: lastly, Rust -- another relatively new/growing language -- has these two conversion functions in its Duration type

Side-effects

If these are added, it might lead to suggestions for other similarly time.go functions:

What version of Go are you using (go version)?

go version go1.11 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

time.Since(time.Now()).Milliseconds()

What did you expect to see?

Some value representing milliseconds elapsed.

What did you see instead?

Compilation error: "...Milliseconds undefined (type time.Duration has no field or method Milliseconds)"

@tiegz tiegz changed the title time: add Duration is missing Milliseconds() and Microseconds() getters #5491 time: add Duration-conversion functions Milliseconds() and Microseconds() Nov 2, 2018
@ianlancetaylor
Copy link
Contributor

Also proposed and rejected in #18935 and #27782. The latter issue recommends creating a third party library to see if it is popular. Has anything changed since those earlier discussions?

@tiegz
Copy link
Author

tiegz commented Nov 3, 2018

@ianlancetaylor those were for different functions, but I mentioned them above too. The idea of a library to measure popularity is interesting, but I don't see how people would know it exists unless they come across these GH issues (which probably explains the low Star count on https://github.com/ulikunitz/unixtime).

Has anything changed since those earlier discussions?

Not that I know of; my main motivation was to lay out a case for why these should exist.

@dmitshur dmitshur added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. FeatureRequest labels Nov 4, 2018
@dmitshur dmitshur added this to the Unplanned milestone Nov 4, 2018
@agnivade agnivade changed the title time: add Duration-conversion functions Milliseconds() and Microseconds() proposal: time: add Duration-conversion functions Milliseconds() and Microseconds() Nov 6, 2018
@rsc
Copy link
Contributor

rsc commented Nov 28, 2018

I'm sympathetic, since it's so easy to mistype the division (e.g. you have to remember NOT to type t.Nanoseconds() / 1000 for milliseconds).
My concern in 2013 in #5491 was the return type.
If you call Micro/Milli, do you most often want it in int64 or float64?
Do other language APIs agree on which to use?

The way you get float would be t.Seconds() * 1000 which is easy to remember.
Maybe that's an argument for providing the integer form, since that's the one with the unexpected constant in the calculation.

@andybons
Copy link
Member

andybons commented Dec 5, 2018

Per discussion with @golang/proposal-review, we agree this should be added. @griesemer suggests int64 as the return type (you can always get the fractional by multiplying the time.Second output). This detail can be determined during review, however.

@andybons andybons changed the title proposal: time: add Duration-conversion functions Milliseconds() and Microseconds() time: add Duration-conversion functions Milliseconds() and Microseconds() Dec 5, 2018
@andybons andybons added Proposal-Accepted NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Dec 5, 2018
@andybons andybons modified the milestones: Unplanned, Go1.13 Dec 5, 2018
@tiegz
Copy link
Author

tiegz commented Dec 5, 2018

Thanks @andybons !

Just to answer @rsc 's question, I mostly picked float64 to be consistent with Seconds(), Minutes(), and Hours(). int64 would be nice since you could just search-and-replace duration / time.Millisecond with duration.Milliseconds() though, so it's a toss-up.

dphan72 added a commit to dphan72/go that referenced this issue Mar 14, 2019
The return values are integers, as opposed to floats, since the fractionals can be derived from multiplying t.Seconds().

Fixes golang#28564
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/167387 mentions this issue: time: add methods to convert duration to microseconds and milliseconds

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FeatureRequest FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Projects
None yet
Development

No branches or pull requests

6 participants