Is an empty flag a bad habit?

Question

I faced a very stupid bug a few days ago. It was caused by this enum I get from third-party library:

[Flags]
public enum MyStatus
{
    OKResponse = 0,
    ResponseTooBig = 1,
    ErrorMessage = 2,
    NoResponse = 4,
    ...
}

I am used to check flags this way:

if ((status & MyStatus.OKResponse) != 0) {...}

but it doesn't work for MyStatus.OKResponse, because it is zero. It is not a flag at all, it is an absence of all flags. Of course, when I found the bug, I realised OKResponse was the only non error status, so it really means "no errors, no flags". However, I really don't find it obvious.

Is it a bad habit defining 0 as one of values in flags enum? What is the recommended way? What is the best way to check flags, that would work with "no flags" flag, too?

Not only is it not a bad habit, the opposite is true - it's bad to NOT define a zero value. This is because the default value for an enum is 0. The convention is to call it `None`. Also, when using `[Flags]` the convention is to pluralise the name of the enum. It looks like your enum should be called something like `Errors`. — Matthew Watson, Mar 07 '16 at 09:50
It seems wrong the use of a Flags enum for this. Do you have a status that is represented by more than one bit? — Steve, Mar 07 '16 at 09:52
@Steve Yes, I had to anonymize the example a little... The first one is OKResponse and the rest are some error flags, more than one possible. Using of Flags is OK, really. — vojta, Mar 07 '16 at 09:53
@MatthewWatson So you think it just does not follow naming convention? — vojta, Mar 07 '16 at 09:55
Note that if you add a `None = 0` to the enum, your check becomes `if (status != Errors.None)` which is quite readable. (And I think you need to add a `None` member and rename the enum to follow [the enum naming convention](https://msdn.microsoft.com/en-us/library/4x252001%28v=vs.71%29.aspx?f=255&MSPPError=-2147217396).) — Matthew Watson, Mar 07 '16 at 09:55
@Jodrell Yes, of course. But I have to know that `MyStatus.OKResponse` is a special value... Should I really check all enums in all libraries I get? — vojta, Mar 07 '16 at 09:56
@vojta actually that might be a good idea. You can't really trust every library to follow conventions. — kapex, Mar 07 '16 at 09:59

Yuval Itzchakov · Accepted Answer · 2016-03-07T10:09:45.950

6

Is it a bad habit defining 0 as one of values in flags enum?

No, on the contrary, as the comments say, it is common to use 0 as a value for a given flag, and that is what an enum will default to if no different value is assigned to the first given value. As others said in the comments, it's also common to use Enum.None as the first value for an enum, which makes your intentions clearer to anyone else reading the code.

What is the recommended way?

There is no one way to do it, but I usually like to use the concise Enum.HasFlag method:

void Main()
{
    var status = MyStatus.ResponseTooBig | MyStatus.NoResponse;
    if (status.Equals(MyStatus.OKResponse))
        Console.WriteLine("Status is OKResponse");
    else 
        Console.WriteLine($"Has NoResponse?: {status.HasFlag(MyStatus.NoResponse)}");
}

edited Mar 07 '16 at 10:09

answered Mar 07 '16 at 09:55

Yuval Itzchakov

146,575
32
257
321

The problem is that `status.HasFlag(MyStatus.OKResponse)` is always true, so it does not work in this case... See http://stackoverflow.com/questions/15436616/hasflags-always-returns-true-for-none-0-value-in-enum – vojta Mar 07 '16 at 10:03
@vojta The solution is in the same post: *If the underlying value of flag is zero, the method returns true. If this behavior is not desirable, you can use the Equals method to test for equality with zero and call HasFlag only if the underlying value of flag is non-zero, as the following example illustrates.* – Yuval Itzchakov Mar 07 '16 at 10:04
Yes, I know. But your example writes `true`, which is not very good - there is a `ResponseTooBig` error flag, right? – vojta Mar 07 '16 at 10:05

score 1 · Answer 2 · answered Mar 07 '16 at 10:31

While it's a good idea to actually define a 0 value in your enum, it's a very bad idea for it to represent OK, because 0 is the default value and it is the only special integer that is implicitly convertible to an enum, so it can creep in easily:

MyStatus status = 0;

So the actual defining of a 0 value is not a problem, but it's a good idea to make it to represent a None for Flags enums or an Invalid value for normal enums.

Also, Flags enums should be used when the members can be combined, but from your example it seems they are mutually exclusive.

So I'd say the enum is badly designed, because as it is, any response has an OK flag and that's certainly a bug for a Flags enum, and if you can't change the definition, the only way would be to explicitly check if the status equals OKResponse:

if (status == MyStatus.OKResponse)

Is an empty flag a bad habit?

2 Answers2