Bitwise flags and Switch statement?

Question

I have the following code (example), and I'm really not comfortable with so many 'if' checks:

public enum Flags 
{
    p1 = 0x01,  // 0001
    p2 = 0x02,  // 0010  
    p3 = 0x04,  // 0100
    p4 = 0x08   // 1000
};      

public static void MyMethod (Flags flag)
{
    if ((flag & Flags.p1) == Flags.p1)
        DoSomething();

    if ((flag & Flags.p2) == Flags.p2)
        DosomethingElse();

    if ((flag & Flags.p3) == Flags.p3)
        DosomethingElseAgain();

    if ((flag & Flags.p4) == Flags.p4)
        DosomethingElseAgainAndAgain();
}

MyMethod(Flags.p1 | Flags.p3);

Is there some way, that i could use an 'switch' statement. Maybe if I convert them to strings, or use arrays?

A `switch` statement is not what you want because it's equivalent to a whole bunch of `if ... else if ... else if ...` statements, which is clearly not what you have. — Gabe, Oct 02 '10 at 18:25
Note that in C# 4.0 you can use `flag.HasFlag(Flags.p1)` instead of `flag & Flags.p1 == Flags.p1` — Gabe, Oct 02 '10 at 18:29
I think the series of if checks is quite clear, as it describes exactly what you're doing. By separating the logic for each section into its own method, you've eliminated the noise that would obscure the top-level pattern, which is a series of conditional actions. — Dan Bryant, Oct 02 '10 at 18:58
Dan Bryant: For the most part you're right, but it's hard to look at it and know that all flags are represented and it's hard to make sure that a maintenance programmer won't come in and start trying to "optimize" it by inserting `else` s in there. — Gabe, Oct 02 '10 at 21:06
The only thing "wrong" with this code is the absence of a nice long comment. — dbasnett, Oct 03 '10 at 14:40

Ani · Accepted Answer · 2010-10-02T18:48:20.320

10

Something like this?

public static void MyMethod(Flags flag)
{
    // Define action-lookup
    var actionsByFlag = new Dictionary<Flags, Action>
    {
        { Flags.p1, DoSomething},
        { Flags.p2, DosomethingElse},
        { Flags.p3, DosomethingElseAgain},
        { Flags.p4, DosomethingElseAgainAndAgain},
    };

    // Find applicable actions
    var actions = actionsByFlag.Where(kvp => (flag & kvp.Key) == kvp.Key)
                               .Select(kvp => kvp.Value);

    //Execute applicable actions
    foreach (var action in actions)
       action();
}

EDIT: If the order of actions are important, an OrderBy clause may be required.

edited Oct 02 '10 at 18:48

answered Oct 02 '10 at 18:26

Ani

111,048
26
262
307

6

In C# 4.0 you can use `dict.Where(kv => flag.HasFlag(kv.Key))` – Callum Rogers Oct 02 '10 at 18:30
I like that @Ani, Dictionary and LINQ. Thanks to @Gabe and @Callum Rogers for mentioning new C# 4.0 feature. – kofucii Oct 02 '10 at 18:34
1

Good answer, but define action-lookup *outside* the method. – Caspar Kleijne Oct 02 '10 at 18:35
Caspar: The action lookup may require state that's only available inside that method. If not, though, it should be defined outside the method. – Gabe Oct 02 '10 at 18:43
This is nice, but, to be honest, I think this is harder to read than the simple series of tests and methods. This version requires that the maintaining developer understand a number of intermediate-level concepts and (of lesser concern), it takes considerably longer to execute. As long as the logic for each flag case is delegated to helper methods, I think the initial implementation is actually quite clean. – Dan Bryant Oct 02 '10 at 18:53
@Dan Bryant: I agree with you; my answer actually has more code than the initial implementation. I do think the dictionary approach is more readable after you've seen the pattern once, though. I would use it if a) performance were not a concern b) there were several (many more than 4 anyway) flags. – Ani Oct 02 '10 at 18:59

score 7 · Answer 2 · edited May 23 '17 at 10:28

Here's a variation on Ani's answer:

public static void MyMethod(Flags flag) 
{ 
    // Define action-lookup 
    var dict = new Dictionary<Flags, Action> 
    { 
        { Flags.p1, DoSomething}, 
        { Flags.p2, DosomethingElse}, 
        { Flags.p3, DosomethingElseAgain}, 
        { Flags.p4, DosomethingElseAgainAndAgain}, 
    }; 

    // Find applicable actions 
    var actions = from value in Enum.GetValues(typeof(Flags))
                  where flag.HasFlag(value)
                  select dict[value];

    //Execute applicable actions 
    foreach (var action in actions)
       action(); 
}

The important difference here is that it iterates over the defined values in the enumeration rather than the entries in the dictionary. That way if you add a new flag to the enum without adding it to the dictionary, you will get an exception when you try to use the new flag. And it always iterates in order of the flags.

Bitwise flags and Switch statement?

2 Answers2

Linked