Monday, April 25, 2011

Enum as Flag using, setting and shifting

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
}
From stackoverflow
  • 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
    };
    
    abatishchev : Main remark is about default value is A | B. So just appending using |= is not enough.
    Can Berk Güder : I updated my answer.
    abatishchev : I agree about ugliness of that code, look please at head post, what do you think?
    Can Berk Güder : 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.
  • 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! :-)

    abatishchev : Thanks, I'll look at &=~ ! But disagree with you about having null-flag. MSDN recommends do not! http://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."
    danbystrom : 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!
  • 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
    }
    

0 comments:

Post a Comment