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 Time.Compare #50770

Closed
jdemeyer opened this issue Jan 24, 2022 · 19 comments
Closed

time: add Time.Compare #50770

jdemeyer opened this issue Jan 24, 2022 · 19 comments

Comments

@jdemeyer
Copy link

There are currently 3 methods to compare time.Time objects: Before(), Equal() and After(). These are analogous to <, == and >. But the equivalents of <= and >= are missing. I suggest adding BeforeEqual() and AfterEqual() to complete this.

I am fully aware that these new methods are not strictly needed: t.BeforeEqual(u) could be written as either t.Before(u) || t.Equal(u) or !t.After(u) or t.Sub(u) <= 0. However, those three are less readable than a simple and obvious t.BeforeEqual(u). I have written code like

for day := begin; /* day <= end */ !day.After(end); day = day.Add(24 * time.Hour) {

where I felt that the comment was needed to clarify the intent.

Implementation of the proposed methods is trivial.

@gopherbot gopherbot added this to the Proposal milestone Jan 24, 2022
@ALTree
Copy link
Member

ALTree commented Jan 24, 2022

At least to me, !t.After( ) looks significantly more readable than BeforeEqual. "not after t" is a very natural way to express the concept of "at the moment t or before". And it's hard to justify increasing the API surface to add a function that is precisely the boolean negation of an existing one.

@jdemeyer
Copy link
Author

jdemeyer commented Jan 24, 2022

At least to me, !t.After( ) looks significantly more readable than BeforeEqual.

Really? If you wanted to write a for loop with x going from a to b, including both endpoints, would you write it like

for x := a; x <= b; x++ {

or

for x := a; !(x>b); x++ {

My guess would be that most people would write it the first way. And I don't really see why it should suddenly be different if x is a time.Time instead of a number. At least this was the rationale for my proposal.

@bcmills
Copy link
Contributor

bcmills commented Jan 24, 2022

If I wanted to write a for loop with x going from a to b in most languages, I would write it as a range.
In most languages, the range would be over the interval [a, b+1), because most languages use half-open intervals for ranges.
(To my dismay, Go does not yet support ranges for user-defined types.)

If I wanted to write a for loop over a sequence of days in Go, I would not use the 3-part loop form. (For me, the 3-part loop form is almost always less clear than some alternative.) Instead, I would write:

	x := a
	for !x.After(b) {
		…
		x = x.AddDate(0, 0, 1)
	}

I think that form is clearer because it emphasizes the semantics: x is increased until it is after b, regardless of whether b happens to be an integer number of days away from a. (I think the use of time.Date to represent entire days — rather than nanoseconds — is awkward even at best; compare #19700.)

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jan 24, 2022
@ericlagergren
Copy link
Contributor

ericlagergren commented Jan 25, 2022

My guess would be that most people would write it the first way. And I don't really see why it should suddenly be different if x is a time.Time instead of a number. At least this was the rationale for my proposal.

In English, anyway, dates are usually compared using "before" and "after" while numbers are usually compared using "greater" or "less." It's why < is the less-than operator, not the before-than operator.

@jdemeyer
Copy link
Author

I think that form is clearer because it emphasizes the semantics: x is increased until it is after b

Right, but x.BeforeEqual(b) also emphasizes semantics the same way: the loop continues as long as x is before or equal to b. I just find the negation in the condition !x.After(b) slightly harder to understand: loops are often more naturally expressed as a "while" condition instead of an "until" condition.

In English, anyway, dates are usually compared using "before" and "after" while numbers are usually compared using "greater" or "less." It's why < is the less-than operator, not the before-than operator.

Right, but that's just a matter of wording: "before" has the same meaning as "less". In languages with operator overloading, you would likely use < for both.

My point is just that it's often natural to write X <= Y for numbers (I checked the Go sources and there are many examples of for loops with a <= condition). And it should be equally natural to want to write "timestamp X is before or equal to timestamp Y".

@rsc
Copy link
Contributor

rsc commented Jan 27, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals (old) Jan 27, 2022
@rogpeppe
Copy link
Contributor

rogpeppe commented Jan 27, 2022

I like this proposal. Getting ordering sense right is often tricky, and the lack of these methods means that I've often needed to write out an expression in terms of how I'm thinking about it, and then change the method and add the negation. It might just be me, but I find that extra negations make it harder to think about correctness, particularly when is trying to concentrate on the details of an algorithm.

However, another possibility might be to add a Compare method:

// Compare returns -1 if t1 is before t2, 0 if t1 equals t2 or 1 if t1 is after t2.
func (t1 Time) Compare(t2 Time) int

This is not only fairly easy to read (x.BeforeEqual(y) becomes x.Compare(y) <= 0
but could have additional utility with the advent of generics (see #50340 (comment) for an example), and save a comparison when using a time as one component of a larger comparison.

@ianlancetaylor
Copy link
Contributor

Considering that time.Time values have nanosecond precision, I'm curious how often the difference between time.Before and time.BeforeEqual matters in practice.

@rogpeppe
Copy link
Contributor

@ianlancetaylor It can matter surprisingly often in my experience - for example, a time might well be exactly equal before you've slept at all and the value is what you start with, or when you're not using time.Now at all but taking items from a heap or using a faked-out time source.

@jdemeyer
Copy link
Author

However, another possibility might be to add a Compare method:

Sure, if that's the direction that Go is moving to. It's very explicit and it would also solve my use case.

Considering that time.Time values have nanosecond precision

As @rogpeppe mentioned, not all time.Time values are "random". They might be timestamps entered in some user interface or they might be timestamps coming from a lower-precision source.

@rsc
Copy link
Contributor

rsc commented Feb 2, 2022

I wasn't very enthusiastic about adding BeforeEqual and AfterEqual but adding Compare seems reasonable and fits in with the other places in the library that define a Compare. Retitling.

Does anyone object to adding Time.Compare?

@rsc rsc changed the title proposal: time: Time.BeforeEqual() and Time.AfterEqual() proposal: time: add Time.Compare Feb 2, 2022
@rogpeppe
Copy link
Contributor

rogpeppe commented Feb 3, 2022

Given that it's so simple to do, I just added https://go-review.googlesource.com/c/go/+/382734 to implement this.

@rsc
Copy link
Contributor

rsc commented Feb 9, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Feb 9, 2022
@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Feb 16, 2022
@rsc
Copy link
Contributor

rsc commented Feb 16, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: time: add Time.Compare time: add Time.Compare Feb 16, 2022
@rsc rsc modified the milestones: Proposal, Backlog Feb 16, 2022
@hopehook
Copy link
Member

Hi, @rogpeppe
I'm interested in this proposal and I noticed that this CL stuck for a while.
Thanks for your great work and hope you keep pushing it.

@gopherbot
Copy link

Change https://go.dev/cl/382734 mentions this issue: time: implement Compare method

@rogpeppe
Copy link
Contributor

@hopehook thanks for the prompt. I'd forgotten about the CL entirely! I've made the appropriate changes now.

@Delta456
Copy link

Delta456 commented Dec 8, 2022

Wouldn't it be better to add constants like time.Past for -1, time.Future for +1 and time.Same for 0? I believe they are much better and more readable than those having hardcore values.

@ianlancetaylor
Copy link
Contributor

@Delta456 We didn't do that for bytes.Compare or strings.Compare. I don't think we have to do it here.

@golang golang locked and limited conversation to collaborators Dec 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

10 participants