Enum as Flag using, setting and shifting

Question

I have two flags:

[Flags]
enum Flags
{
  A = 1,
  B = 2
};

I set them like this:

Mode = Flags.A | Flags.B; // default value
for(int i = 0; i < args.Length; i++) {
switch(args[i])
{
  case "--a":
  {
    if ((Mode & Flags.A) == Flags.A && (Mode & Flags.B) == Flags.B) 
     // both, default assume
    {
      Mode = Flags.A; // only A
    }
    else
    {
      Mode |= Flags.A; // append A
    }
    break;
  }
  case "--b":
  {
    if ((Mode & Flags.A) == Flags.A && (Mode & Flags.B) == Mode.B)
    {
      Mode = Flags.B;
    }
    else
    {
      Mode |= Flags.B;
    }
    break;
  }
} }

and use them later like this:

if((Mode & Flags.A) == Flags.A)
{
 //
}
if((Mode & Flags.B) == Flags.B)
{
 //
}

Major reservation: Both flags may be set. Or just one, in this case only one part of code is executed.

Is my code good? What is the best way to setup flags?

Upd: Is this less ugly then first, how do you think?

Mode = 0; // default value now is empty
for(int i = 0; i < args.Length; i++) {
switch(args[i])
{
  case "--a":
  {
    Mode |= Flags.A;
    break;
  }
  case "--b":
  {
    Mode |= Flags.B;
    break;
  }
} }
if(Mode == 0)
{
  Mode = Flags.A | Flags.B; // if no parameters are given, setup both flags
}

Barth

Your code is syntactically correct but its hard to say whether it's "good" without understanding your problem a bit more. Your question is a bit vague and your comments on answers make it more so. Can you give a bit more? On the face of it your solution is too complex for the problem as presented. – Psychopathist

Hi. There is default value A|B. If parameter --a is given set mode only to A, if --b only to B, and if both are given to (as default) A|B. What is the best way to check is current value is default (A|B) to change it to A or B if required. – Barth

Answer 1

Here's how I would set my flags:

Mode = 0;
for(int i = 0; i < args.Length; i++) {
    switch(args[i]) {
    case "--a":
        Mode |= Flags.A;
        break;
    case "--b":
        Mode |= Flags.B;
        break;
    }
}

If both flags should be on by default, I think it makes more sense to change the command line parameters to something like --not-a and --not-b. This would both reflect the default setting, and let you get rid of (Mode & Flags.A) == Flags.A && (Mode & Flags.B) == Flags.B, which is rather ugly, IMHO.

Then you can set your flags like this:

Mode = Flags.A | Flags.B;
for(int i = 0; i < args.Length; i++) {
    switch(args[i]) {
    case "--not-a":
        Mode &= ~Flags.A;
        break;
    case "--not-b":
        Mode &= ~Flags.B;
        break;
    }
}

Finally, if you have a lot of flags (instead of just two), it might be easier to set up your enum like this:

[Flags]
enum Flags
{
    A = 1,
    B = 1 << 1,
    C = 1 << 2,
    D = 1 << 3,
    E = 1 << 4,
    F = 1 << 5
};

Tower

Main remark is about default value is A | B. So just appending using |= is not enough. – Barth

I agree about ugliness of that code, look please at head post, what do you think? – Barth

The new code is much better, but I still think --not-a and --not-b make more sense. On the other hand, Mode will never be null (you set it to 0), so you should check for Mode == 0. – Venitavenite

Answer 2

You can turn a "bit" off with the following wonderful statement:

Mode &= ~Flags.A;

I'd reccoment including a "null" value in your enum as well:

[Flags]
enum Flags
{
  Null = 0;
  A = 1,
  B = 2;
}

It will keep your life simpler! :-)

Decolorant

Thanks, I'll look at &=~ ! But disagree with you about having null-flag. MSDN recommends do not! msdn.microsoft.com/en-us/library/ms229062.aspx "There is no way to check for a zero value flag being explicitly set, as opposed to no flags being set." – Barth

It is not for setting any individual bit. It is for clearing and giving a defined default value. And it is easier to test if a bit is set by typing if ( (Mode & Flags.A) != Flags.Null. There are many reasons why yo're life will be simpler with a .Null value. Trust me on this one! – Lentiginous

I agree with having a zero value for a flags-style enum, but Microsoft recommends the name None, not Null - as mentioned in the document @Barth links to. – Imes

Answer 3

The second version is much better - this is exactly what I would do. Change Mode == null to Mode == 0 though.

Mode = 0; // default value now is empty
for(int i = 0; i < args.Length; i++) {
    switch(args[i])
    {
        case "--a":
            Mode |= Flags.A;
            break;

        case "--b":
            Mode |= Flags.B;
            break;
    }
}

if(Mode == 0)
{
    Mode = Flags.A | Flags.B; // if no parameters are given, setup both flags
}

Ossie