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

Consistent capitalisation of enum values/constants #7270

Open
dwightwatson opened this issue Jan 5, 2019 · 16 comments
Open

Consistent capitalisation of enum values/constants #7270

dwightwatson opened this issue Jan 5, 2019 · 16 comments

Comments

@dwightwatson
Copy link
Contributor

Rolling on from some discussion in #7247 it's been pointed out that as it stands there is no convention for the naming of values on an enum. Through the Crystal code base there are both examples of SNAKE_UPPERCASE and PascalCase implementations.

module Colorize
  enum ColorANSI
    Default = 39
    Black = 30
    # ...
  end
end
class CSV::Builder
  enum Quoting
    NONE
    RFC
    # ...
  end
end

I think it would be ideal if a convention could be established here for the consistency of the codebase.

One option I see as preferable is that constants will be SNAKE_UPPERCASE and that enum values will be PascalCase. One additional benefit from this would be possible (generally) to infer if a value is a constant or an enum from its capitalisation.

For example, Crypto::Blowfish::DEFAULT_ROUNDS would be a constant, but Colorize::ColorANSI::Default would be an enum.

Related, and not sure if it's possible at all, but if a convention was decided on it would be great if the Crystal formatter could automatically handle the change.

@straight-shoota
Copy link
Member

Enum members are semantically like constants, so I don't follow why they should have different naming conventions.

Choosing one style or the other is mostly a question of preference and heritage/sentimentality (others might call it religion). Languages in the Java family typically use UPPERCASE for enum members, whereas C# and related languages use PascalCase. Of the more modern languages, Rust and Go advocate PascalCase, Kotlin accepts both. (These mentions are only referring to the stdlib naming conventions not what the respective language supports)

Crystal, too has previously accepted both, but PascalCase is more dominant. UPPERCASE is mostly but not exclusively used when creating an enum to represent grouped C constants.

While being mostly equivalent, there are a few practical differences:

  • PascalCase is actually less expressive because not all characters have upper and lower case variants. These characters are unable to denote word boundaries which can lead to ambiguities. In pure PascalCase, an enum member like X86_64 would be X8664 which means loosing detail. Or it would need to insert an underscore after all (could also be a different separator).
  • UPPER_SNAKE_CASE aligns perfectly with lower_snake_case which is used in the auto generated predicate methods on enum types. Following up on the previous point this avoids any confusion whether the respective predicate method for Foo1 is #foo1? or #foo_1?.
  • Enums are often used for mapping C constants, which are usually UPPPERCASED. Thus it is natural to follow the same naming convention for the equivalent enum members and not having to care how to transcribe to PascalCase.

A common argument against UPPERCASE is that it is screaming and abrasive. While I don't agree to that, I understand that other people might perceive it as such (I can relate to that, as my typografic aesthetics are infuriated by PascalCasing all-cap acronyms). But in the end, I don't think it matters that much. Between autocasted enum values and predicate methods, the actual member names are rarely used outside the enum definition. And in that capacity UPPER_SNAKE_CASE has the practical advantage of similarity to the lower_snake_case names.

Hence, I prefer an UPPERCASE naming convention for enum members.

@RX14
Copy link
Contributor

RX14 commented Jan 5, 2019

  • Colorize uses PascalCase
  • Logger uses CAPS
  • Regex::Options uses CAPS (because PCRE does), but few people interact with that enum
  • Process uses PascalCase
  • UUID uses PascalCase
  • IO uses PascalCase
  • Socket uses CAPS (because C does), but few people interact with the socket type enums
  • Time uses PascalCase
  • CSV uses PascalCase and CAPS
  • File::Info uses PascalCase
  • Flate/Gzip uses CAPS (because libz does), but few people interact with those enums either
  • HTTP uses PascalCase
  • LLVM uses a lot of enums, all PascalCase
  • Unicode uses PascalCase
  • YAML uses CAPS
  • WebSocket uses CAPS, but not in user-facing code
  • OpenSSL uses PascalCase

It seems that switching everything to CAPS would cause a lot more breaking changes, in a lot more commonly-used code, than switching everything to PascalCase. So since the arguments for either are largely subjective, I suggest we go with the easiest change. The largest breaking change would be to Logger.

@Sija
Copy link
Contributor

Sija commented Jan 5, 2019

@RX14 Loosing detail in conversion is pretty objective argument. I agree with that too.

@asterite
Copy link
Member

asterite commented Jan 5, 2019

Also remember that the autogenerated flag enum members are None and All. PascalCase is what I expect enums to use.

@dwightwatson
Copy link
Contributor Author

I was a fan of @straight-shoota's argument for going all caps - especially in regard to the automatic generation of predicate methods - however having None and All by default sure throws a spanner in the works.

@asterite
Copy link
Member

asterite commented Jan 6, 2019

But then also know that I would personally remove those generated constants. I regret that decision and we are still in time to remove them.

@Sija
Copy link
Contributor

Sija commented Jan 6, 2019

I regret that decision and we are still in time to remove them.

@asterite what makes you regret it?

@RX14
Copy link
Contributor

RX14 commented Jan 7, 2019

@asterite you don't want to remove the generated constants because of their case though.

@asterite
Copy link
Member

asterite commented Jan 7, 2019

@asterite what makes you regret it?

When you have a flags enum members, for example Read and Write, you generally ask whether it's read and do something, or write and do something. I can't see a need for having None and All. And if they are needed you can just write them down, with a meaningful name. Maybe All could be ReadWrite in this case. And maybe None doesn't make sense if it's for a file open mode (just an example).

@Sija
Copy link
Contributor

Sija commented Jan 7, 2019

Better example of flags enum members would be rather Italic, Bold and Underline, in which having All makes a perfect sense to me.

@straight-shoota
Copy link
Member

Let's continue the discussion about autogenerated enum members in #7285

@straight-shoota
Copy link
Member

straight-shoota commented Apr 2, 2020

This has been brought up again in #8984, but this issue is older and has already more comments on the topic.

I'd point out that there are several ideas about changing enum that could also impact the naming of members: symbols vs. enum (#6736), None and All (#7285), predicate methods (#8000), issues with name collisions (#7084) and ideas from literal types (https://forum.crystal-lang.org/t/literal-types-like-in-typescript/1510). We should take potential consequences from those ideas into consideration (at least whether/how we want to persue them or not).
Effectively, I wouldn't want to define any style guide for enums before settling on these issues.

UPDATE: It seems I already declared #7285 settled, so we're just waiting for None and All to be removed.

@asterite
Copy link
Member

asterite commented Apr 2, 2020

@straight-shoota Since PascalCase was voted unanimously, can we go forward with that? What would be the consequences of, say, going forward with PascalCase? Would any of the other issues benefit if we go with SNAKE_CASE? Is it just that SNAKE_CASE is similar to the question method that's generated for enums?

@straight-shoota
Copy link
Member

straight-shoota commented Apr 2, 2020

What would be the consequences of, say, going forward with PascalCase?

I don't know. But that's the thing. We should solve the important questions first. I wouldn't want to push a decision now on a topic that really doesn't matter much whether it goes one way or the other and has worked so far with diversive case styles.

crush-157 added a commit to crush-157/crystal-book that referenced this issue May 6, 2020
The approved style for enum values is CamelCase, as per crystal-lang/crystal#7270

However this is not stated in the style guide, although the approved style for constants is.
@j8r
Copy link
Contributor

j8r commented Jun 28, 2020

I think the decision of the case should be related to the short-band one, to have consistency.

enum E
  FirstElement
  SECOND_ELEMENT
end

def m(e : E)
end

# All already valid
m :FirstElement
m :SECOND_ELEMENT
m :second_element

For now the status-quo is :snake_case, which is close but not really UPPER_CASE.
It makes sense to have the short-band be the last part of the enum type, like :FirstElement for E::FirstElement, or :SECOND_ELEMENT for E::SECOND_ELEMENT.

Do we prefer to see :PascalCase or :UPPER_CASE?

@MrSorcus
Copy link
Contributor

enum Protocol
 IPv4
 IPv6
end

with this code crystal docs generates i_pv4? and i_pv6?.

enum Protocol
  IPV4
  IPV6
end

generates ipv4? and ipv6? which is more pretty.
Maybe add annotation for control this converting? 🤔

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

No branches or pull requests

8 participants