You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@arrow.apache.org by Weston Pace <we...@gmail.com> on 2021/06/04 19:34:40 UTC
[C++] [DISCUSS] Moving towards a consistent enum naming scheme
The C++ code base currently has a mix of ALL_CAPS (e.g.
arrow::ValueDescr::Shape, seems to be favored in arrow::compute::),
CapWords (e.g. arrow::StatusCode), and kCapWords (e.g.
arrow::DecimalStatus, not common in arrow:: but used in gandiva:: and
technically what the Google style guide recommends[1]).
I don't know that it is worth fixing any existing code but perhaps we
should be consistent going forwards? In a recent PR of mine Antoine
pointed out that ALL_CAPS can clash with macro names. That seems
reasonable so my preference would be AllCaps.
If we can reach some consensus I'll put a PR together to update the
style guide docs
[1] https://google.github.io/styleguide/cppguide.html#Enumerator_Names
-Weston
Re: [C++] [DISCUSS] Moving towards a consistent enum naming scheme
Posted by Micah Kornfield <em...@gmail.com>.
The style guide doesn't make a distinction [1] between scoped and
unscoped. Ultimately, I'm happy to go with whatever the consensus is but
one of the reasons for adopting a style guide is to limit discussions like
this when there are multiple reasonable paths forward.
[1] https://google.github.io/styleguide/cppguide.html#Enumerator_Names
On Fri, Jun 4, 2021 at 12:43 PM Antoine Pitrou <an...@python.org> wrote:
>
> We're talking about new enums here, which should always be scoped enums,
> i.e.:
>
> enum class MyEnum { <some enum values> };
>
>
>
> Le 04/06/2021 à 21:41, Wes McKinney a écrit :
> > I think we should use the kCapWords naming scheme for global
> > const/constexpr values and for bare (non-scoped) enum values. For
> > example:
> >
> > enum MyEnum {
> > kMyEnum_Value1,
> > kMyEnum_Value2,
> > };
> >
> > but not
> >
> > struct MyEnum {
> > enum type {
> > ...
> > };
> > };
> >
> > Does that seem like a reasonable distinction? I think the objective is
> > to be able to reason about constant values at the namespace level
> >
> > On Fri, Jun 4, 2021 at 2:39 PM Micah Kornfield <em...@gmail.com>
> wrote:
> >>
> >> I would prefer kCamelCaps to be inline with the style guide (unless we
> are
> >> too far down a different path).
> >>
> >> On Fri, Jun 4, 2021 at 12:37 PM Antoine Pitrou <an...@python.org>
> wrote:
> >>
> >>>
> >>> Le 04/06/2021 à 21:34, Weston Pace a écrit :
> >>>> The C++ code base currently has a mix of ALL_CAPS (e.g.
> >>>> arrow::ValueDescr::Shape, seems to be favored in arrow::compute::),
> >>>> CapWords (e.g. arrow::StatusCode), and kCapWords (e.g.
> >>>> arrow::DecimalStatus, not common in arrow:: but used in gandiva:: and
> >>>> technically what the Google style guide recommends[1]).
> >>>>
> >>>> I don't know that it is worth fixing any existing code but perhaps we
> >>>> should be consistent going forwards? In a recent PR of mine Antoine
> >>>> pointed out that ALL_CAPS can clash with macro names. That seems
> >>>> reasonable so my preference would be AllCaps.
> >>>
> >>> I'm in favor of CamelCaps as well.
> >>>
> >>> Regards
> >>>
> >>> Antoine.
> >>>
>
Re: [C++] [DISCUSS] Moving towards a consistent enum naming scheme
Posted by Benjamin Kietzman <be...@gmail.com>.
Relevant to this thread: https://github.com/apache/arrow/pull/10691
adds an enum replacement which includes compile-time to/from strings.
This should reduce repetitive boilerplate in the bindings and elsewhere
mapping to/from user provided string values.
On Mon, Jun 14, 2021 at 11:17 PM Micah Kornfield <em...@gmail.com>
wrote:
> >
> > In light of this my preference would be: public enums must be scoped and
> > named in CamelCase. The style guide would require kCamelCase but it does
> so
> > for consistency with public _unscoped_ enums. IMO we'd be better served
> by
> > requiring scoped enums, which obviates the 'k' prefix.
>
>
> This seems to just add more rules to remember for not that much gain (and
> an additional exception to the style guide).
>
> On Fri, Jun 4, 2021 at 8:48 PM Benjamin Kietzman <be...@gmail.com>
> wrote:
>
> > My two cents:
> >
> > kSomeConstant includes the prefix 'k' so that regardless of what
> expression
> > it appears in, it is unambiguously a global constant. I'm restating the
> > obvious here to clarify that the goal is making names obviously global.
> >
> > Note that a prefix is redundant for any qualified name (appears to the
> > right of ::). For example, Type::Boolean unambiguously refers to a
> global-
> > static data member or a scoped enum value, since 'Type' is not a legal
> > namespace.
> >
> > In light of this my preference would be: public enums must be scoped and
> > named in CamelCase. The style guide would require kCamelCase but it does
> so
> > for consistency with public _unscoped_ enums. IMO we'd be better served
> by
> > requiring scoped enums, which obviates the 'k' prefix.
> >
> > Static data members may be referenced without qualification within the
> > class' scope, so to ensure they are obviously global even there the 'k'
> > prefix should still be applied.
> >
>
Re: [C++] [DISCUSS] Moving towards a consistent enum naming scheme
Posted by Micah Kornfield <em...@gmail.com>.
>
> In light of this my preference would be: public enums must be scoped and
> named in CamelCase. The style guide would require kCamelCase but it does so
> for consistency with public _unscoped_ enums. IMO we'd be better served by
> requiring scoped enums, which obviates the 'k' prefix.
This seems to just add more rules to remember for not that much gain (and
an additional exception to the style guide).
On Fri, Jun 4, 2021 at 8:48 PM Benjamin Kietzman <be...@gmail.com>
wrote:
> My two cents:
>
> kSomeConstant includes the prefix 'k' so that regardless of what expression
> it appears in, it is unambiguously a global constant. I'm restating the
> obvious here to clarify that the goal is making names obviously global.
>
> Note that a prefix is redundant for any qualified name (appears to the
> right of ::). For example, Type::Boolean unambiguously refers to a global-
> static data member or a scoped enum value, since 'Type' is not a legal
> namespace.
>
> In light of this my preference would be: public enums must be scoped and
> named in CamelCase. The style guide would require kCamelCase but it does so
> for consistency with public _unscoped_ enums. IMO we'd be better served by
> requiring scoped enums, which obviates the 'k' prefix.
>
> Static data members may be referenced without qualification within the
> class' scope, so to ensure they are obviously global even there the 'k'
> prefix should still be applied.
>
Re: [C++] [DISCUSS] Moving towards a consistent enum naming scheme
Posted by Benjamin Kietzman <be...@gmail.com>.
My two cents:
kSomeConstant includes the prefix 'k' so that regardless of what expression
it appears in, it is unambiguously a global constant. I'm restating the
obvious here to clarify that the goal is making names obviously global.
Note that a prefix is redundant for any qualified name (appears to the
right of ::). For example, Type::Boolean unambiguously refers to a global-
static data member or a scoped enum value, since 'Type' is not a legal
namespace.
In light of this my preference would be: public enums must be scoped and
named in CamelCase. The style guide would require kCamelCase but it does so
for consistency with public _unscoped_ enums. IMO we'd be better served by
requiring scoped enums, which obviates the 'k' prefix.
Static data members may be referenced without qualification within the
class' scope, so to ensure they are obviously global even there the 'k'
prefix should still be applied.
Re: [C++] [DISCUSS] Moving towards a consistent enum naming scheme
Posted by Antoine Pitrou <an...@python.org>.
We're talking about new enums here, which should always be scoped enums,
i.e.:
enum class MyEnum { <some enum values> };
Le 04/06/2021 à 21:41, Wes McKinney a écrit :
> I think we should use the kCapWords naming scheme for global
> const/constexpr values and for bare (non-scoped) enum values. For
> example:
>
> enum MyEnum {
> kMyEnum_Value1,
> kMyEnum_Value2,
> };
>
> but not
>
> struct MyEnum {
> enum type {
> ...
> };
> };
>
> Does that seem like a reasonable distinction? I think the objective is
> to be able to reason about constant values at the namespace level
>
> On Fri, Jun 4, 2021 at 2:39 PM Micah Kornfield <em...@gmail.com> wrote:
>>
>> I would prefer kCamelCaps to be inline with the style guide (unless we are
>> too far down a different path).
>>
>> On Fri, Jun 4, 2021 at 12:37 PM Antoine Pitrou <an...@python.org> wrote:
>>
>>>
>>> Le 04/06/2021 à 21:34, Weston Pace a écrit :
>>>> The C++ code base currently has a mix of ALL_CAPS (e.g.
>>>> arrow::ValueDescr::Shape, seems to be favored in arrow::compute::),
>>>> CapWords (e.g. arrow::StatusCode), and kCapWords (e.g.
>>>> arrow::DecimalStatus, not common in arrow:: but used in gandiva:: and
>>>> technically what the Google style guide recommends[1]).
>>>>
>>>> I don't know that it is worth fixing any existing code but perhaps we
>>>> should be consistent going forwards? In a recent PR of mine Antoine
>>>> pointed out that ALL_CAPS can clash with macro names. That seems
>>>> reasonable so my preference would be AllCaps.
>>>
>>> I'm in favor of CamelCaps as well.
>>>
>>> Regards
>>>
>>> Antoine.
>>>
Re: [C++] [DISCUSS] Moving towards a consistent enum naming scheme
Posted by Wes McKinney <we...@gmail.com>.
I think we should use the kCapWords naming scheme for global
const/constexpr values and for bare (non-scoped) enum values. For
example:
enum MyEnum {
kMyEnum_Value1,
kMyEnum_Value2,
};
but not
struct MyEnum {
enum type {
...
};
};
Does that seem like a reasonable distinction? I think the objective is
to be able to reason about constant values at the namespace level
On Fri, Jun 4, 2021 at 2:39 PM Micah Kornfield <em...@gmail.com> wrote:
>
> I would prefer kCamelCaps to be inline with the style guide (unless we are
> too far down a different path).
>
> On Fri, Jun 4, 2021 at 12:37 PM Antoine Pitrou <an...@python.org> wrote:
>
> >
> > Le 04/06/2021 à 21:34, Weston Pace a écrit :
> > > The C++ code base currently has a mix of ALL_CAPS (e.g.
> > > arrow::ValueDescr::Shape, seems to be favored in arrow::compute::),
> > > CapWords (e.g. arrow::StatusCode), and kCapWords (e.g.
> > > arrow::DecimalStatus, not common in arrow:: but used in gandiva:: and
> > > technically what the Google style guide recommends[1]).
> > >
> > > I don't know that it is worth fixing any existing code but perhaps we
> > > should be consistent going forwards? In a recent PR of mine Antoine
> > > pointed out that ALL_CAPS can clash with macro names. That seems
> > > reasonable so my preference would be AllCaps.
> >
> > I'm in favor of CamelCaps as well.
> >
> > Regards
> >
> > Antoine.
> >
Re: [C++] [DISCUSS] Moving towards a consistent enum naming scheme
Posted by Micah Kornfield <em...@gmail.com>.
I would prefer kCamelCaps to be inline with the style guide (unless we are
too far down a different path).
On Fri, Jun 4, 2021 at 12:37 PM Antoine Pitrou <an...@python.org> wrote:
>
> Le 04/06/2021 à 21:34, Weston Pace a écrit :
> > The C++ code base currently has a mix of ALL_CAPS (e.g.
> > arrow::ValueDescr::Shape, seems to be favored in arrow::compute::),
> > CapWords (e.g. arrow::StatusCode), and kCapWords (e.g.
> > arrow::DecimalStatus, not common in arrow:: but used in gandiva:: and
> > technically what the Google style guide recommends[1]).
> >
> > I don't know that it is worth fixing any existing code but perhaps we
> > should be consistent going forwards? In a recent PR of mine Antoine
> > pointed out that ALL_CAPS can clash with macro names. That seems
> > reasonable so my preference would be AllCaps.
>
> I'm in favor of CamelCaps as well.
>
> Regards
>
> Antoine.
>
Re: [C++] [DISCUSS] Moving towards a consistent enum naming scheme
Posted by Antoine Pitrou <an...@python.org>.
Le 04/06/2021 à 21:34, Weston Pace a écrit :
> The C++ code base currently has a mix of ALL_CAPS (e.g.
> arrow::ValueDescr::Shape, seems to be favored in arrow::compute::),
> CapWords (e.g. arrow::StatusCode), and kCapWords (e.g.
> arrow::DecimalStatus, not common in arrow:: but used in gandiva:: and
> technically what the Google style guide recommends[1]).
>
> I don't know that it is worth fixing any existing code but perhaps we
> should be consistent going forwards? In a recent PR of mine Antoine
> pointed out that ALL_CAPS can clash with macro names. That seems
> reasonable so my preference would be AllCaps.
I'm in favor of CamelCaps as well.
Regards
Antoine.