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

[RFC] Rich enum constants without parens #136

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexkaratarakis
Copy link
Collaborator

No description provided.

@@ -364,14 +365,59 @@ class SkeletalRichEnumStorageBase<RichEnumType,

} // namespace fixed_containers::rich_enums_detail

template <typename RichEnumType>
class RichEnumConstantProxy
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally approve of this idea. I am leaving my thoughts as a comment rather than as PR review such that replies are simpler.

Specifically, I like that this change makes the common use cases of RichEnums simpler. For example, I like that we can omit operator() when comparing enums such as:

TestRichEnum1::value_of(BE::C_ONE) == TestRichEnum1::C_ONE

or in switch statements such as:

case TestRichEnum1::C_ONE:

However, the current approach has some downsides in my eyes.

  1. The first downside in my opinion is that calling member functions of RichEnums now requires operator->, which in my opinion is an operator which conveys pointer like semantics which imply nullptr possibility. However, since the pointer will never be null in this case, this is a purely aesthetic concern and unfortunately I don't currently have alternate suggestions. Example:
TestRichEnum1::C_FOUR->ordinal()
  1. I am not in favor of the
// TRANSITION
constexpr const RichEnumType& operator()()

In my opinion, this will result in having two different ways of doing the same thing and I expect both to stick around. From what I've seen, things in transition:: tend to stick around ;) . I suggest that we make this a breaking change that requires users to change their code. That is unless this repository has some convention of backwards compatibility.

BackingEnum backing_enum_{};

public:
explicit(false) constexpr RichEnumConstantProxy(const BackingEnum& backing_enum)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question from curiosity: I've noticed the pattern explicit(false) for a few ctors. What is the reason for it? Omitting it would have no effect since ctors are implicit by default, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explicit(false) makes it explicit that the function is intended to allow implicit conversion and it is not an omission.

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 this pull request may close these issues.

2 participants