Check Flags Enum for Validity

[jehugaleahsa]

Enum provides a useful method called IsDefined that I use for checking
for valid enums. However, it doesn't work for Flags enums.

Is there an efficient means of testing a Flags enum to see if it has a
valid value?

 

[Peter Duniho]

Enum provides a useful method called IsDefined that I use for checking
for valid enums. However, it doesn't work for Flags enums.

Is there an efficient means of testing a Flags enum to see if it has a
valid value?

Your problem is poorly defined. What, exactly, counds as a "valid
value"? Flags are specific intended to be combined. Are you looking for
individual flag values that exist in the enum? Or is a "valid value" any
combination of the values defined in the enum? Do you have enum values
that are combinations of other enum values? Or is each enum value
discrete?

Depending on what it is you're really trying to do, it's likely that you
could just create a bitmask, the one's complement of all of your enum
values or'ed with each other, which you can then and with a value to test;
if the result is non-zero, that _might_ mean that the value tested is
invalid, depending on what your definition of "valid" is.

Pete

 

[jehugaleahsa]

Your problem is poorly defined.  What, exactly, counds as a "valid  
value"?  Flags are specific intended to be combined.  Are you lookingfor  
individual flag values that exist in the enum?  Or is a "valid value" any  
combination of the values defined in the enum?  Do you have enum values 
that are combinations of other enum values?  Or is each enum value  
discrete?

Depending on what it is you're really trying to do, it's likely that you  
could just create a bitmask, the one's complement of all of your enum  
values or'ed with each other, which you can then and with a value to test;  
if the result is non-zero, that _might_ mean that the value tested is  
invalid, depending on what your definition of "valid" is.

Pete

By Flags enums, I was referring to the FlagsAttribute. You use that
when your enum allows its values to be "or"-ed together.

Let's use the age-old file access example with an enum that looks
something like this:

[Flags]
enum FileAccess
{
Read,
Write
}

Now, say you have a method that take a FileAccess.

void MyMethod(FileAccess access)
{
if (!Enum.Defined(typeof(FileAccess), access))
{
throw new ArgumentException("The file access was not a defined
value.", "access");
}
...
}

This will fail if someone passes FileAccess.Read | FileAccess.Write.

I want to check if the value is a "possible" value. So in the example,
the sample input should be fine. I could create a third enum value or
ReadWrite = Read | Write, but that doesn't always make sense for some
enums.

So, it there an efficient way of doing a check?

 

[Peter Duniho]

By Flags enums, I was referring to the FlagsAttribute. You use that
when your enum allows its values to be "or"-ed together.

Um. I know what a "flags enum" is, thank you.

[...]
if (!Enum.Defined(typeof(FileAccess), access))
{
throw new ArgumentException("The file access was not a defined
value.", "access");
}
...
}

This will fail if someone passes FileAccess.Read | FileAccess.Write.

I want to check if the value is a "possible" value. So in the example,
the sample input should be fine. I could create a third enum value or
ReadWrite = Read | Write, but that doesn't always make sense for some
enums.

So, it there an efficient way of doing a check?

Yes. I described it in my previous reply.

One possible implementation would be to simply define a
"FileAccess.InvalidBits" in the enum (fixing the "Read = 0" error you had
earlier):

[Flags]
enum FileAccess
{
Read = 1,
Write = 2,
InvalidBits = ~(Read | Write)
}

Then:

if ((access & FileAccess.InvalidBits) != 0)
{
throw new ArgumentException("...");
}

As you say "that doesn't always make sense for some enums", that phrase
could apply to a number of solutions. So, you have to be specific about
what exact problem you're trying to solve before it can be stated whether
there's an efficient solution or not. For the specific problem you've
stated in your example, the above should work.

Pete

 

[jehugaleahsa]

By Flags enums, I was referring to the FlagsAttribute. You use that
when your enum allows its values to be "or"-ed together.

Um.  I know what a "flags enum" is, thank you.

[...]
    if (!Enum.Defined(typeof(FileAccess), access))
    {
        throw new ArgumentException("The file access was not a defined
value.", "access");
    }
    ...
}

This will fail if someone passes FileAccess.Read | FileAccess.Write.

I want to check if the value is a "possible" value. So in the example,
the sample input should be fine. I could create a third enum value or
ReadWrite = Read | Write, but that doesn't always make sense for some
enums.

So, it there an efficient way of doing a check?

Yes.  I described it in my previous reply.

One possible implementation would be to simply define a  
"FileAccess.InvalidBits" in the enum (fixing the "Read = 0" error you had  
earlier):

     [Flags]
     enum FileAccess
     {
         Read = 1,
         Write = 2,
         InvalidBits = ~(Read | Write)
     }

Then:

     if ((access & FileAccess.InvalidBits) != 0)
     {
         throw new ArgumentException("...");
     }

As you say "that doesn't always make sense for some enums", that phrase  
could apply to a number of solutions.  So, you have to be specific about  
what exact problem you're trying to solve before it can be stated whether 
there's an efficient solution or not.  For the specific problem you've  
stated in your example, the above should work.

Pete- Hide quoted text -

- Show quoted text -

I knew you knew. I just wasn't sure you knew I knew. Just trying to be
precise. ;-)

One enum in our system allows the users to specify which days or the
week an item is valid. So, they can pick Monday through Sunday or
bitwise-or multiple days together. I want to make sure that someone
doesn't try to pass my methods a value like 567891234 or something
else rediculous. I also don't think creating a separate enum value for
each day combination is worth the effort.

I could always code the values of my enums and do an integer range
check. I didn't know if there was a "built-in" means of accomplishing
the same thing.

 

[Peter Duniho]

[...] I want to make sure that someone
doesn't try to pass my methods a value like 567891234 or something
else rediculous. I also don't think creating a separate enum value for
each day combination is worth the effort.

I could always code the values of my enums and do an integer range
check. I didn't know if there was a "built-in" means of accomplishing
the same thing.

There is. I've described it. Twice. Including a code example. At
least, that's as "built-in" a solution as I know of.

You can, of course, calculate the special "InvalidBits" value at run-time,
rather than making it part of the enum. Then you don't even have to worry
about updating the "InvalidBits" value if you add an enum value. But it
wouldn't be nearly as succinct a solution as what I showed.

Pete

 

[Arne Vajhøj]

Enum provides a useful method called IsDefined that I use for checking
for valid enums. However, it doesn't work for Flags enums.

Is there an efficient means of testing a Flags enum to see if it has a
valid value?

Unless this operation is very performance critical, then I will
suggest something like:

public static class FlagHack
{
public static bool IsFlagDefined(this Enum en)
{
int allbits = 0;
foreach(int bit in Enum.GetValues(en.GetType()))
{
allbits |= bit;
}
int val = Convert.ToInt32(en);
return (val & allbits) == val;
}
}

because that will be a lot easier to maintain.

Arne

 

[jehugaleahsa]

[...] I want to make sure that someone
doesn't try to pass my methods a value like 567891234 or something
else rediculous. I also don't think creating a separate enum value for
each day combination is worth the effort.

I could always code the values of my enums and do an integer range
check. I didn't know if there was a "built-in" means of accomplishing
the same thing.

There is.  I've described it.  Twice.  Including a code example.  At  
least, that's as "built-in" a solution as I know of.

You can, of course, calculate the special "InvalidBits" value at run-time,  
rather than making it part of the enum.  Then you don't even have to worry  
about updating the "InvalidBits" value if you add an enum value.  But it  
wouldn't be nearly as succinct a solution as what I showed.

Pete

Maybe I'm missing something. When you are creating the InvalidBits,
are you creating all of the bits that if set mean that the value is
invalid?

If so, I guess what you are saying makes perfect sense. I didn't
understand that at first. It essentially does the same thing Arne
suggested, but with a single operation.

I wish there was a way to create the InvalidBits without creating an
actual enum value.

Sorry I didn't see what you were saying earlier.

 

[Ben Voigt [C++ MVP]]

Unless this operation is very performance critical, then I will
suggest something like:

public static class FlagHack
{
public static bool IsFlagDefined(this Enum en)
{
int allbits = 0;
foreach(int bit in Enum.GetValues(en.GetType()))
{
allbits |= bit;
}
int val = Convert.ToInt32(en);
return (val & allbits) == val;

This test is wrong, Peter's comparison to zero is right.

}
}

because that will be a lot easier to maintain.

A little better performance and more flexible because it will work with
enums of any underlying type (but untested, I think there's a cast necessary
following the compile step on that expression tree being used to provide the
delegate for the fold operation):

public static class InvalidBits<T> : InvalidBits where T : struct /* C#
doesn't allow the constraint where T: enum */
{
public static readonly UInt64 All;

static InvalidBits()
{
Type etype = typeof(T);
if (!etype.IsEnum) throw InvalidOperationException();
ParameterExpression[] foldArgs = {
Expression.Parameter(typeof(UInt64)),
Expression.Parameter(typeof(object)) };
All = ~Enumerable<object, UInt64>.Aggregate(Enum.GetValues(etype), 0,
Expression.Lambda(Expression.Or(foldArgs(1), Expression.Unbox(foldArgs(2),
Enum.GetUnderlyingType(etype))), foldArgs).Compile());
}
}


Then use the & operator and test the result against zero as Peter suggests.

 

[Peter Duniho]

Maybe I'm missing something. When you are creating the InvalidBits,
are you creating all of the bits that if set mean that the value is
invalid?

Yes...each bit set in the InvalidBits value is a bit that does not appear
in any of the valid values. Thus, if one or more of those bits are set,
you've got an invalid value.

In my example, the programmer is required to maintain the value
explicitly. There's zero performance penalty, and it's a lot less code,
but it does have the maintenance hazard I mentioned earlier.

The other approach I mentioned is elaborated on by Arne and Ben. Ben's
example sure is fancy , but I find Arne's more readable (if you fix the
little mistake about the final comparison against "val" versus "0").

I admit, I don't really understand why Ben's example goes to so much
trouble, what with messing around with the expressiont tree and all. Even
if I were going to use the Enumerable.Aggregate() method, I think I would
just use it in the conventional way, simply providing a delegate that or's
the previous value with the current value. But knowing Ben, I'm sure
there's a method to his madness.

If so, I guess what you are saying makes perfect sense. I didn't
understand that at first. It essentially does the same thing Arne
suggested, but with a single operation.

I wish there was a way to create the InvalidBits without creating an
actual enum value.

Sure, there is. Just do it programmatically like Arne and Ben suggest.

Pete

 

[Ben Voigt [C++ MVP]]

Yes...each bit set in the InvalidBits value is a bit that does not appear
in any of the valid values. Thus, if one or more of those bits are set,
you've got an invalid value.

In my example, the programmer is required to maintain the value
explicitly. There's zero performance penalty, and it's a lot less code,
but it does have the maintenance hazard I mentioned earlier.

The other approach I mentioned is elaborated on by Arne and Ben. Ben's
example sure is fancy , but I find Arne's more readable (if you fix the
little mistake about the final comparison against "val" versus "0").

I admit, I don't really understand why Ben's example goes to so much
trouble, what with messing around with the expressiont tree and all. Even
if I were going to use the Enumerable.Aggregate() method, I think I would
just use it in the conventional way, simply providing a delegate that or's
the previous value with the current value. But knowing Ben, I'm sure
there's a method to his madness.

There's a reason for the madness, maybe not the best one -- GetValue returns
an object[] which contains boxed values. These boxed values all have the
type Enum.GetUnderlyingType(etype), which may not be Int32 and may not fit
in Int32 (anything from byte/sbyte all the way up to ulong/long are
possible). Arne's method only supports enums based on int (the default).

It probably would be desirable to cache the delegate created by the
expression tree, there are only eight different possible underlying types

 

[Peter Duniho]

There's a reason for the madness, maybe not the best one -- GetValue
returns an object[] which contains boxed values. These boxed values all
have the type Enum.GetUnderlyingType(etype), which may not be Int32 and
may not fit in Int32 (anything from byte/sbyte all the way up to
ulong/long are possible). Arne's method only supports enums based on
int (the default).

I'm still not sure I comprehend the need for the expression tree, versus
just writing some imperative algorithm. But I'm also sitting around
waiting for news on a family medical emergency, so I admit I'm probably
not putting as much effort into comprehension as I might ordinarily do.
I'm guessing that by taking advantage of expression trees, you're able to
imbed the actual type in the calculation, rather than having to cast to
some lowest-common-denominator type that can handle any enum?

Anyway, I figured there was some specific reason, even if it wasn't
immediately apparent to me.

It probably would be desirable to cache the delegate created by the
expression tree, there are only eight different possible underlying
types and so using a Dictionary<Type, Func<X,Y,Z>> would be sufficient.

Probably. Though personally, I think I wouldn't even worry too much about
a generic solution. The frequency that this might come up is probably low
enough that one might as well just write a type-specific loop, and that's
assuming you need validation anyway (personally, when I'm using flags, I
generally just write code that ignores bits that aren't relevant...I
believe .NET follows that approach generally speaking as well).

Pete

 

[Ben Voigt [C++ MVP]]

There's a reason for the madness, maybe not the best one -- GetValue
returns an object[] which contains boxed values. These boxed values all
have the type Enum.GetUnderlyingType(etype), which may not be Int32 and
may not fit in Int32 (anything from byte/sbyte all the way up to
ulong/long are possible). Arne's method only supports enums based on
int (the default).

I'm still not sure I comprehend the need for the expression tree, versus
just writing some imperative algorithm. But I'm also sitting around
waiting for news on a family medical emergency, so I admit I'm probably
not putting as much effort into comprehension as I might ordinarily do.
I'm guessing that by taking advantage of expression trees, you're able to
imbed the actual type in the calculation, rather than having to cast to

That's pretty much the reason.

some lowest-common-denominator type that can handle any enum?

There is no such type. When unboxing value types from object, you must
specify the exact type.

Anyway, I figured there was some specific reason, even if it wasn't
immediately apparent to me.


Probably. Though personally, I think I wouldn't even worry too much about
a generic solution. The frequency that this might come up is probably low
enough that one might as well just write a type-specific loop, and that's
assuming you need validation anyway (personally, when I'm using flags, I
generally just write code that ignores bits that aren't relevant...I
believe .NET follows that approach generally speaking as well).

True. Selecting a lambda from eight possible, based on the result of
GetUnderlyingType, should still be an order of magnitude faster than using
the Convert class which will do a runtime type check on each and every
element of the array.

Something then like:

public static class InvalidBits<T> : InvalidBits where T : struct /* C#
doesn't allow the constraint where T: enum */
{
public static readonly ulong All;

static InvalidBits()
{
Type etype = typeof(T);
if (!etype.IsEnum) throw InvalidOperationException();
Type utype = Enum.GetUnderlyingType(etype);
Func<ulong,object,ulong> fold_func;
if (utype == typeof(byte))
fold_func = (ulong accum, object elem) => accum | (byte)elem;
else if (utype == typeof(sbyte))
fold_func = (ulong accum, object elem) => accum | (sbyte)elem;
// ...
else if (utype == typeof(ulong))
fold_func = (ulong accum, object elem) => accum | (ulong)elem;
else throw new NotImplementedException();

All = ~Enumerable<object, ulong>.Aggregate(Enum.GetValues(etype), 0,
fold_func);
}
}

 

[Jeff Johnson]

Enum provides a useful method called IsDefined that I use for checking
for valid enums. However, it doesn't work for Flags enums.

Is there an efficient means of testing a Flags enum to see if it has a
valid value?

For an enum with the Flags attribute, your code generally walks through
various bit patterns looking for specific flags. Therefore, if extra bits
are set which don't correspond to a defined value, WHO CARES? You won't be
looking for them in the first place, right? In other words, I don't
understand the real problem. It's like you're walking through the bits and
trying to match them to known flags, instead of walking through the known
flags to see if their bits are set. Why would you do it the first way?

 

[Ben Voigt [C++ MVP]]

Enum provides a useful method called IsDefined that I use for checking
for valid enums. However, it doesn't work for Flags enums.

Is there an efficient means of testing a Flags enum to see if it has a
valid value?

For an enum with the Flags attribute, your code generally walks through
various bit patterns looking for specific flags. Therefore, if extra bits
are set which don't correspond to a defined value, WHO CARES? You won't be
looking for them in the first place, right? In other words, I don't
understand the real problem. It's like you're walking through the bits and
trying to match them to known flags, instead of walking through the known
flags to see if their bits are set. Why would you do it the first way?
[/QUOTE]

Because it could be the only visible sign of another error? Or of a hacker
fuzzing a public-facing API? Ignoring unreasonable values until they cause
a real problem is not a very good approach to building robustness.

 

[jehugaleahsa]

Because it could be the only visible sign of another error?  Or of a hacker
fuzzing a public-facing API?  Ignoring unreasonable values until they cause
a real problem is not a very good approach to building robustness.- Hide quoted text -

- Show quoted text -

Exactly. This is one of those instances where I generally don't worry
about it. Until recently I didn't.

The problem was that we added another value to one of our Flag enums.
However, prior to it being changed, there was a piece of code passing
in a bogus bit. Until the old code started failing tests, we had no
idea there was a bug. This happened because of the way we were
generating the enum at runtime. We've fixed the bug but I wanted to
create a regression test for the code that had the bug. I also
thought, if it was efficient, I would start checking my methods for
valid input.

I think I will hold off checking my arguments. I will use Arne's
(corrected) check in my regression tests though. It is pretty simple
and I'll know if it ends up being something other than an int when the
bar turns red. (That's also why I didn't want to use original Pete's
solution because it would force me to change production code.)

Thanks for a rather enlightening discussion.