Policy for style guide: document use of 'e' prefixed enums in C/C++ #98553

New Issue

Campbell Barton · 2022-06-02T09:00:24+02:00

Campbell Barton commented

2022-06-02 09:00:24 +02:00

Currently most typedef'd enums use an e prefix, although this isn't mentioned in the code style guide.

As this isn't noted as a convention it's not clear if/when they should be used if it's reasonable to request them in code-review for example.

I'd propose to include this as part of Blender code-style https://wiki.blender.org/wiki/Source/Code_Style - mainly because its following what's already being done (for the most-part), and it can be a useful hint.

When used, it's obvious the name is an enum type which are often passed and copied by value and don't need to be declared const in the header for e.g. Where as all other types we normally pass by pointer or reference and declare const where possible.

From a quick search in source/:

typedef enum e[A-Z] + ^enum e[A-Z] -> 817.
typedef enum [A-Z] + ^enum [A-Z] -> 185.

This misses some indented declarations but I don't think it makes a big difference.

Currently most typedef'd `enums` use an `e` prefix, although this isn't mentioned in the code style guide. As this isn't noted as a convention it's not clear if/when they should be used if it's reasonable to request them in code-review for example. I'd propose to include this as part of Blender code-style https://wiki.blender.org/wiki/Source/Code_Style - mainly because its following what's already being done (for the most-part), and it can be a useful hint. ---- When used, it's obvious the name is an `enum` type which are often passed and copied by value and don't need to be declared `const` in the header for e.g. Where as all other types we normally pass by pointer or reference and declare `const` where possible. ---- From a quick search in `source/`: - `typedef enum e[A-Z]` + `^enum e[A-Z]` -> 817. - `typedef enum [A-Z]` + `^enum [A-Z]` -> 185. *This misses some indented declarations but I don't think it makes a big difference.*

Campbell Barton commented

2022-06-02 09:00:24 +02:00

Added subscriber: @ideasman42

Campbell Barton changed title from ~~Policy for style guide: use of 'e' prefixed enums~~ to Policy for style guide: use of 'e' prefixed enums in C/C++

2022-06-02 09:05:23 +02:00

Campbell Barton changed title from ~~Policy for style guide: use of 'e' prefixed enums in C/C++~~ to Policy for style guide: document use of 'e' prefixed enums in C/C++

2022-06-02 09:31:48 +02:00

Hans Goudey commented

2022-06-02 09:48:46 +02:00

Added subscriber: @HooglyBoogly

Hans Goudey commented

2022-06-02 09:48:46 +02:00

Personally I would prefer not so use the e prefix for enums, for a few reasons:

In practice it's almost always clear that something is an enum anyway, either because of context, or words like Type, Mode, Flag etc. in the enum name.
"e" makes the enum names harder to say, which is a really important way to improve code readability IMO.
A bit more fuzzy: it makes the code look less like "normal language", which makes it more intimidating.
An extra letter-- makes code longer
Lack of first letter capital CamelCase for types (nice for consistency)

For the pass-by-value/const things meantioned in the task, I'm not sure this is much different from other custom types that we also pass by value (Span, IndexMask, etc.)
Avoiding enum names that look like struct names makes sense, but personally I don't think this is the best way to do it, and I could see it even making enum names worse.

It doesn't change much, but since typedef isn't usually used in C++, I think the searches above misses some examples:

enum [A-Z] -> 188
enum class [A-Z] -> 47

Personally I would prefer not so use the `e` prefix for enums, for a few reasons: - In practice it's almost always clear that something is an enum anyway, either because of context, or words like `Type`, `Mode`, `Flag` etc. in the enum name. - "e" makes the enum names harder to say, which is a really important way to improve code readability IMO. - A bit more fuzzy: it makes the code look less like "normal language", which makes it more intimidating. - An extra letter-- makes code longer - Lack of first letter capital CamelCase for types (nice for consistency) --- For the pass-by-value/const things meantioned in the task, I'm not sure this is much different from other custom types that we also pass by value (`Span`, `IndexMask`, etc.) Avoiding enum names that look like struct names makes sense, but personally I don't think this is the best way to do it, and I could see it even making enum names worse. --- It doesn't change much, but since `typedef` isn't usually used in C++, I think the searches above misses some examples: `enum [A-Z]` -> 188 `enum class [A-Z]` -> 47

Sergey Sharybin commented

2022-06-02 09:56:27 +02:00

Added subscriber: @Sergey

Sergey Sharybin commented

2022-06-02 09:56:27 +02:00

Something we've been talking with Campbell in the blender chat. I share the thoughts with Hans here.

To not repeat what is already mentioned, the gist would be: to me the important part is to make the "meaningful" part of the type name clear. Using Flag / Type whenever the enum is a flag or a type of something I find to be more important.

Something we've been talking with Campbell in the blender chat. I share the thoughts with Hans here. To not repeat what is already mentioned, the gist would be: to me the important part is to make the "meaningful" part of the type name clear. Using `Flag` / `Type` whenever the enum is a flag or a type of something I find to be more important.

Bastien Montagne commented

2022-06-02 09:58:20 +02:00

Added subscriber: @mont29

Bastien Montagne commented

2022-06-02 09:58:20 +02:00

Not really convinced here, don't really see the added value to that rule, not a big fan of adding type hints in names in general... and would rather avoid yet another giant cleanup messing up with tens of header files.

So no hard no from me, but a 'would rather not' ;)

Not really convinced here, don't really see the added value to that rule, not a big fan of adding type hints in names in general... and would rather avoid yet another giant cleanup messing up with tens of header files. So no hard no from me, but a 'would rather not' ;)

Jacques Lucke commented

2022-06-02 10:22:52 +02:00

Added subscriber: @JacquesLucke

Jacques Lucke commented

2022-06-02 10:22:52 +02:00

Generally agree with Hans, Sergey and Bastien here. Don't have another argument to add.

Sybren A. Stüvel commented

2022-06-02 11:01:28 +02:00

Added subscriber: @dr.sybren

Sybren A. Stüvel commented

2022-06-02 11:01:28 +02:00

Same here, I agree with Hans, Sergey, and Bastien. For me, personally, these enum-related things are much more important for understandability of the code:

Better documentation of short and int as this-should-be-an-enum-type in DNA, so that it's clear which enum is used (this is sometimes already there, but also sometimes not).
IMO using int and short for enum values in non-DNA code should be forbidden.
Distinction between "enum of bit flags that can be ORed into a single variable" vs. "actual enum that can take only a single one of those values".

IMO if you need an e prefix for an enum, it's a sign that the declaration is too far from the use, and that the code is likely getting too complex. That's not a problem to be solved with a prefix.

PS: the above is not intended to change the scope of this patch. Just to elaborate a bit on my vision on the use of enums in our code.

Same here, I agree with Hans, Sergey, and Bastien. For me, personally, these enum-related things are much more important for understandability of the code: - Better documentation of `short` and `int` as this-should-be-an-enum-type in DNA, so that it's clear which enum is used (this is sometimes already there, but also sometimes not). - IMO using `int` and `short` for enum values in non-DNA code should be forbidden. - Distinction between "enum of bit flags that can be ORed into a single variable" vs. "actual enum that can take only a single one of those values". IMO if you need an `e` prefix for an enum, it's a sign that the declaration is too far from the use, and that the code is likely getting too complex. That's not a problem to be solved with a prefix. PS: the above is not intended to change the scope of this patch. Just to elaborate a bit on my vision on the use of enums in our code.

Campbell Barton commented

2022-06-22 07:28:24 +02:00

As the replies here prefer not to use the e prefix I've created a counter proposal #99080 (Policy for style guide: document enum suffix use for C/C++).

As the replies here prefer not to use the `e` prefix I've created a counter proposal #99080 (Policy for style guide: document enum suffix use for C/C++).

Philipp Oeser removed the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Policy for style guide: document use of 'e' prefixed enums in C/C++ #98553