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

Added indexable properties on enums #59

Merged
merged 14 commits into from
Sep 6, 2018
Merged

Conversation

Shelim
Copy link
Contributor

@Shelim Shelim commented Aug 31, 2018

  • _to_index which always return value 0... size-1
    (even if enums are not sequential)
  • _from_index used for other way around
  • _from_index_nothrow no throw version
  • _from_index_unchecked returns invalid enum
    in case arg>=size

Code and test cases added.
No documentation updates (my English is too poor for that)

Usage case:

BETTER_ENUM(Enum, int, A = 128, B = 256, C = 1024)
for(int i = 0; i < Enum::_size(); i++)
{
    Enum foo = Enum::_from_index_unchecked(i);
    printf("%d enum is %s \n", i, foo._to_string());
}

Shelim and others added 2 commits August 31, 2018 16:42
 - _to_index which return value 0... size-1
        (even if enums are note sequential)
 - _from_index used for other way around
 - _from_index_nothrow no throw version
 - _from_index_unchecked returns invalid enum
        in case arg>=size

Code and test cases added.
No documentation updates (my English is too poor for that)
Added indexable properties on enums
@Shelim
Copy link
Contributor Author

Shelim commented Aug 31, 2018

Sorry, older version pushed for some reason o.O I'll fix that asap

@Shelim
Copy link
Contributor Author

Shelim commented Aug 31, 2018

Uff... I believe it is done now. Hard weekend...

@Shelim
Copy link
Contributor Author

Shelim commented Aug 31, 2018

Fail is in clang-3.3 and is not related to that code

Copy link
Owner

@aantron aantron left a comment

Choose a reason for hiding this comment

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

At first glance, it looks like we should expose _from_value_loop, which is what the new _to_index does. For the rest of the functions, the same functionality can be used by the user with:

MyEnum::_values()[index]

and the user can implement their own error handling for the case where the index is out of range, since there is public MyEnum::_size().

Does that serve your needs?

If so, I would suggest cutting this PR down to include only _to_index, by making _from_value_loop pubic and renaming it. In the documentation for _to_index, we can tell users how to perform the reverse operation by using _values()[index].

enum.h Outdated
@@ -700,6 +717,15 @@ Enum::_from_value_loop(Enum::_integral value, std::size_t index) \
_from_value_loop(value, index + 1); \
} \
\
BETTER_ENUMS_CONSTEXPR_ inline Enum::_optional \
Enum::_from_index_loop(std::size_t index) \
Copy link
Owner

Choose a reason for hiding this comment

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

I recommend not calling this ..._loop, because it doesn't loop over the indexes, but just does a direct lookup. In fact, it looks like just the implementation of _from_index_nothrow, so I suggest moving it there.

@aantron
Copy link
Owner

aantron commented Aug 31, 2018

Looking more closely at the use case, I think it is completely covered by direct access to _values()[index].

@Shelim
Copy link
Contributor Author

Shelim commented Sep 1, 2018

yup, but I would very much prefer symmetrical API - it is less confusing - if we are to expose method _to_index :)

Another, more pratctial usage case:

    BETTER_ENUM(my_enum, int, A = 65536, B = 65537, C = 536870912)
    std::bitset<my_enum._size()> bitset; // Compact bitset
    bitset.set(+(my_enum::foo)._to_index());

@aantron aantron merged commit 97d38d8 into aantron:master Sep 6, 2018
@aantron
Copy link
Owner

aantron commented Sep 6, 2018

Okay, thanks for the PR!

aantron added a commit that referenced this pull request Oct 19, 2020
aantron added a commit that referenced this pull request Oct 19, 2020
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