ScottishTapWater
ScottishTapWater

Reputation: 4846

Problems with Enum as Flags

I have the following enum:

[Flags]
public enum Status { Nominal, Modified, DirOneOnly, DirTwoOnly, DirOneNewest, DirTwoNewest }

I am attempting to see whether the Modified bit has been set to true and have tried the following methods:

if(_stateFlags.HasFlag(Status.Modified))
{
    //DoStuff
}   //Found out why this doesn't work after reading docs.

and

if((_stateFlags & Status.Modified) == Status.Modified)
{
    //DoStuff
}

The latter is the method that my further research led me to believe would work. However, when I do _stateFlags = Status.DirTwoOnly the above statement still seems to evaluate to true which is really puzzling me.

Am I doing something fundamentally wrong?

Upvotes: 0

Views: 2152

Answers (3)

Eric Lippert
Eric Lippert

Reputation: 660493

You have several answers to your question explaining what is wrong. I would suggest taking a different approach entirely.

Flags enums have a very 1990s feel to them; if you think they look like they're there for COM interop, and if you think COM enums look like they are for compatibility with bit-twiddling code from the 1970s, you're right.

I would not express this logic as an enum in new code. I would take the time to write a custom struct that clearly expresses my actual semantics.

struct Status
{
  public static readonly Status None = default(Status);
  private Status(int bits) { this.bits = bits; }
  private int bits;
  private const int NominalBitMask  = 0b0001;
  private const int ModifiedBitMask = 0b0010;
  ... etc ...
  public bool IsNominal => (this.bits & NominalBitMask) != 0;
  public Status WithNominal(bool f) => 
    new Status(f ? (this.bits | NominalBitMask) : (this.bits & ~NominalBitMask));
  ... etc ...

And now you can use it like:

Status status = Status.None.WithNominal(true).WithModified(myFile.IsModified);
...
if (status.IsModified) ...

Is this more work up front? Sure, it's about twenty minutes more work up front. But you then never make any bit twiddling mistake ever again. You have a struct you can test independently of the logic that uses it. You have code that looks like what it means. You never have to worry about someone casting an integer to your enum type and having it full of nonsense. You can put custom logic in your type; suppose for example there are three-valued flags -- true, false or null, say -- those are hard to do in flags enums but you can add the logic easily in a custom type. And so on.

Upvotes: 7

apocalypse
apocalypse

Reputation: 5884

The code:

[Flags]
public enum Status { Nominal, Modified, DirOneOnly, DirTwoOnly, DirOneNewest, DirTwoNewest }

is equal to:

[Flags]
public enum Status
{ 
    Nominal      = 0,  // in bits: ... 0000 0000
    Modified     = 1,  //              0000 0001
    DirOneOnly   = 2,  //              0000 0010
    DirTwoOnly   = 3,  //              0000 0011 **common bit with Modified state **
    DirOneNewest = 4,
    DirTwoNewest = 5,

}

So as other people said, you need to use power of 2 for enum values.

Upvotes: 1

TVOHM
TVOHM

Reputation: 2742

You need to define the enum constants as powers of two.

[Flags]
public enum Status
{
    Nominal = 1,
    Modified = 2,
    DirOneOnly = 4,
    DirTwoOnly = 8,
    DirOneNewest = 16,
    DirTwoNewest = 32
}

class Program
{
    static void Main(string[] args)
    {
        Status s = new Status();
        s |= Status.Modified;
        if (s.HasFlag(Status.Modified))
        {
            Console.WriteLine("Modified!");
        }
    }
}

Upvotes: 6

Related Questions