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

proposal: cmp: Add constants LessThan, GreaterThan, EqualTo #61593

Closed
earthboundkid opened this issue Jul 26, 2023 · 6 comments
Closed

proposal: cmp: Add constants LessThan, GreaterThan, EqualTo #61593

earthboundkid opened this issue Jul 26, 2023 · 6 comments
Labels
Milestone

Comments

@earthboundkid
Copy link
Contributor

Along the lines of io.SeekStart, add constants to cmp to make the magic numbers more readable:

const (
    LessThan = -1
    EqualTo = 0
    GreaterThan = 1
)
@gopherbot gopherbot added this to the Proposal milestone Jul 26, 2023
@jimmyfrasche
Copy link
Member

my preference would be LT, EQ, GT as these would be fairly common though I understand if that's too terse. I imagine it would mostly be used in switches so case cmp.LT: seems clear enough to me.

@robpike
Copy link
Contributor

robpike commented Jul 26, 2023

I agree with @jimmyfrasche: the proposed names are verbose. I'm not even sure they're necessary, although they're certainly clear. But then giving small integers big names for common uses like seeking and comparison was not the tradition I grew up in; maybe things have moved on.

I would prefer LT, EQ, etc. but then that's Fortran, so who am I to say?

@earthboundkid
Copy link
Contributor Author

I think if it’s short it should be Eq not EQ because the Q isn’t an initial.

@mikeschinkel
Copy link

I prefer what @carlmjohnson proposed.

More effort to type, but unambiguous and thus easier to read.

@rsc
Copy link
Contributor

rsc commented Jul 27, 2023

These names would steer developers away from the most elegant way to use cmp.Compare and related functions. In addition to LT, GT, and EQ, what about defining LE and GE? You can't.

The reason you can't is that these functions are intended to be used with actual operators in a comparison with zero. That is, the idiom is that if you are trying to find out X <op> Y then you write cmp.Compare(X, Y) <op> 0. For example, to check whether X <= Y you write cmp.Compare(X, Y) <= 0. There is no way to do this with a named constant.

We could add the named constants, and then if cmp.Compare(X, Y) == LessThan becomes standard usage, when people start using the == form the natural question arises: how do we check for LessThanOrEqual? It's unclear, you have to invent some other idiom. People will end up writing things like cmp.Compare(X, Y) != GreaterThan instead of the much clearer cmp.Compare(X, Y) <= 0.

We could of course define LT, GT, and EQ for use in comparison implementations, but then they become an attractive nuisance, something that is possibly okay for defining compare functions but definitely inappropriate when using them.

Perhaps the expected usage should be made clearer in the documentation of these functions. I don't believe the constants are a net win.

@earthboundkid
Copy link
Contributor Author

You could write GTE as

switch cmp.Compare(a.Foo, b.Foo) {
case cmp.GT, cmp.Eq:

But I take the point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants