-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add FlagwiseNot(). #11
Conversation
EnumUtilTests/UnitTests.cs
Outdated
Assert.AreEqual(OverlappingValuesFlagsEnum.Two | OverlappingValuesFlagsEnum.Eight, EnumUtil<OverlappingValuesFlagsEnum>.FlagwiseNot(overlappingValue)); | ||
Assert.AreEqual(OverlappingValuesFlagsEnum.All, EnumUtil<OverlappingValuesFlagsEnum>.FlagwiseNot(default)); | ||
Assert.AreEqual(EnumUtil<OverlappingValuesFlagsEnum>.FlagwiseNot(default), OverlappingValuesFlagsEnum.All); | ||
foreach (var anotherOverlappingValue in EnumUtil<OverlappingValuesFlagsEnum>.GetValues()) Assert.AreEqual(anotherOverlappingValue, EnumUtil<OverlappingValuesFlagsEnum>.FlagwiseNot(EnumUtil<OverlappingValuesFlagsEnum>.FlagwiseNot(anotherOverlappingValue))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add brackets for the foreach (foreach (...) { }
) and move the body to its own line
foreach (var flagValue in GetValues()) allSetValue = BitwiseOr(allSetValue, flagValue); | ||
var allSetValueConverted = Expression.Convert(Expression.Constant(allSetValue), Enum.GetUnderlyingType(typeof(TEnum))); | ||
|
||
// (~val)&allSetValue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment could use spaces around the &
for readability
// We can AND this with the bitwise not to restrict the set flags to | ||
// actual flags. | ||
var allSetValue = default(TEnum); | ||
foreach (var flagValue in GetValues()) allSetValue = BitwiseOr(allSetValue, flagValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the body of the foreach loop on its own line and use {}
// We can AND this with the bitwise not to restrict the set flags to | ||
// actual flags. | ||
var allSetValue = default(TEnum); | ||
foreach (var flagValue in GetValues()) allSetValue = BitwiseOr(allSetValue, flagValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know that this would work. As this assumes that during static instantiation BitwiseOr exists and can be accessed when FlagwiseNot is being set.
Feels janky :/
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does need the implementation of BitwiseOr
. It would seem like a waste to temporarily build an extra BitwiseOr
just for it. I think I will update GenerateFlagwiseNot()
to accept an argument which will be an implementation of BitwiseOr
so that the dependency is visible at the call-site. Then it is obvious that the initialization of flagwiseNot
has to be after the initialization of bitwiseOr
.
Oh, this should probably be against InDev branch to ensure that master is always the latest release. |
Convenience to invert all flags on an enum. Fixes arogozine#10.
I cannot figure out how to change the target branch of a PR (maybe that is intentionally impossible). So I have created PR #12 in order to target InDev. |
Convenience to invert all flags on an enum.
Fixes #10.