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

ReferenceEquality implementation vs. documentation: enums #783

Closed
Stephan202 opened this issue Oct 19, 2017 · 0 comments · Fixed by #1120
Closed

ReferenceEquality implementation vs. documentation: enums #783

Stephan202 opened this issue Oct 19, 2017 · 0 comments · Fixed by #1120

Comments

@Stephan202
Copy link
Contributor

As noted in #440, the ReferenceEquality check doesn't flag enum comparisons. The documentation on the other hand seems to claim otherwise:

But enum values are always unique, so can’t I compare them with ==?

Yes, but that might confuse the reader, who must understand that your type has special properties because it’s an enum. Using equals everywhere can work the same everywhere; special-casing for enums isn’t worth it.

I assume (hope) that the documentation is merely outdated. I can open a PR to drop this section, but perhaps it should be replaced with some text explaining that Enum and Class comparisons are excluded. WDYT?

ronshapiro pushed a commit that referenced this issue Sep 17, 2018
…heck them.

I considered rephrasing it to say that using #equals is preferred but not enforced by this check, but mentioning them at all seems confusing.

Fixes #783

RELNOTES: Tweak documentation for ReferenceEquality

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=212956380
@ronshapiro ronshapiro mentioned this issue Sep 17, 2018
cushon pushed a commit that referenced this issue Sep 17, 2018
…heck them.

I considered rephrasing it to say that using #equals is preferred but not enforced by this check, but mentioning them at all seems confusing.

Fixes #783

RELNOTES: Tweak documentation for ReferenceEquality

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=212956380
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant