5

Note the simple example below:

Module Module1
    <Flags>
    Public Enum Names
        None    = 0
        Test    = 1
        Test2   = 2
        Test3   = 4
        Test4   = 8
    End Enum

    Sub Main()
        Dim test As Names = Names.Test Or Names.Test3
        If (test And Names.Test3) = Names.Test3
            Console.WriteLine("TRUE")
        Else
            Console.WriteLine("FALSE")
        End If
    End Sub
End Module

The first part of my question relates to the line If (test And Names.Test3) = Names.Test3.

Wouldn't it be better to simply check If test And Names.Test3 as if the flag exists? If it evaluates to a non-zero value (meaning the flag exists) then the result of the condition would be True anyway.

Is there a good reason to use the first way of checking over the second? (Whilst my answer is for VB.NET, I would also be interested in knowing if this is a potential pitfall anywhere else, ie C#, C++, etc).

Also, with regards to flag removal, it seems there's two ways to do this:

test = test Xor Names.Test3 and test = test And Not Names.Test3

However, the first will add the flag if it's missing, and remove it if it is there, whereas the second will only remove it. Is that the only difference? Or is there another reason why I should prefer one method over the other?

Interminable
  • 1,338
  • 3
  • 21
  • 52
  • We usualy only like 1 question. But for your 2nd, that's the difference for your case. It gets more complicated when you start combining flags (test Xor (Names.Test2 Or Names.Test3). In that case, Xor could create weird result if you don't have both flags on. – the_lotus Jul 05 '16 at 11:34
  • I figured that my questions were minor enough that they shouldn't be split. That `Xor` thing you've flagged up seems to work as expected to me, it just does everything at once. In your example, it adds `Names.Test2`, because it's not there, and removes `Names.Test3`, because it is (already there)! – Interminable Jul 05 '16 at 11:45

2 Answers2

5

You are correct in stating that you can effectively replace this:

If (test And Names.Test3) = Names.Test3 Then

with this

If (test And Names.Test3) Then

But, the second example will not compile with Option Strict On as you rightly get the error:

Option Strict On disallows implicit conversions from 'Names' to 'Boolean' so in order to get this to compile you need to wrap a CBool round it.

So, in conclusion I would say it is much better to use the first example as the intent is very clear:- you are checking to see if a bit is set.

In terms of flag removal i.e. unsetting a bit you should use:

test = test And Not Names.Test3

Using Xor has the effect of toggling the value.

The following might help (especially if you make them extension methods):

Public Function SetBit(ByVal aValue As Names, ByVal aBit As Names) As Names
    Return (aValue Or aBit)
End Function

Public Function ClearBit(ByVal aValue As Names, ByVal aBit As Names) As Names
    Return (aValue And Not aBit)
End Function

Public Function IsBitSet(ByVal aValue As Names, ByVal aBit As Names) As Boolean
    Return ((aValue And aBit) = aBit)
End Function

Public Function ToggleBit(ByVal aValue As Names, ByVal aBit As Names) As Names
    Return (aValue Xor aBit)
End Function
Community
  • 1
  • 1
Matt Wilko
  • 26,994
  • 10
  • 93
  • 143
  • Argh! I normally set both `Option Strict` and `Option Explicit` to `On`, but I'd just created this as a simple test project to gain understanding of bitwise operations....and forgot! Thanks for pointing that out. Definitely a good reason for going with the first method. Do you have any thoughts on the second part of my question regarding flag removal? – Interminable Jul 05 '16 at 11:12
  • 2
    You can *default* Strict On: http://stackoverflow.com/questions/5160669/option-strict-on-by-default-in-vb-net - this should be the out of the box setting. Vote for MS to change this here: https://visualstudio.uservoice.com/forums/121579-visual-studio-2015/suggestions/10672947-set-option-strict-on-by-default-instead-of-off – Matt Wilko Jul 05 '16 at 11:15
  • 1
    I knew about setting it for a project as a whole in the project settings, I didn't know I could make Visual Studio do this by default! Very handy. The idea you linked has my votes. – Interminable Jul 05 '16 at 11:23
  • Wait, are you saying that `test = test And Not Names.Test3` won't work with `Option Strict`? Or did you mean this part: `If test And Names.Test3`? – Visual Vincent Jul 05 '16 at 13:37
  • @VisualVincent - No that is not what I am saying. In your example the `If` statement won't compile – Matt Wilko Jul 05 '16 at 13:39
  • 1
    Oh wait, misread your answer. Haha. Thank you -- I like your answer, though since I've already used 40 votes today I cannot upvote 'til tomorrow. :) – Visual Vincent Jul 05 '16 at 13:41
  • @MattWilko I really like your answer, but do you also have any thoughts on the second part of my question regarding flag removal? – Interminable Jul 05 '16 at 13:42
2

Remember that Flags enums don't have to all be purely single bit values. E.g. imagine (with better names) that your enum was :

<Flags>
Public Enum Names
    None    = 0
    Test    = 1
    Test2   = 2
    Test3   = 4
    Test4   = 8
    Test2AndTest4 = 10
End Enum

Now, you wouldn't want to just test that test And Names.Test2AndTest4 is non-zero since that doesn't answer the correct question. So it's a better habit to get into, in general, to And your mask to check and then compare to the mask value, to ensure that all bits of the mask are set.

Damien_The_Unbeliever
  • 234,701
  • 27
  • 340
  • 448
  • Is this wise? I cannot think of the words to describe what I'm feeling about this, but combining creating new `Enum` entries to combine existing `Enum` entries in the same `Enum` feels a bit off. Also, even if I were to do this, rather than `Test2AndTest4 = 10`, shouldn't I just do `Test2AndTest4 = Names.Test2 Or Names.Test4`? – Interminable Jul 05 '16 at 14:35
  • 1
    @Interminable - when a certain combination of options is going to be exceptionally common, then it's common to give it a more convenient name rather than always forcing bitwise operations at point of use. E.g. in the various `File*` enumerations in `System.IO`, most of them have a specific `ReadWrite` member named. – Damien_The_Unbeliever Jul 05 '16 at 14:38
  • After doing some reading, it looks like this sort of thing may be the exception rather than the rule. If I had a particular flag combination that was definitely going to be re-used a lot, I might consider it, however, right now `ReadWrite` is the only example I've been able to locate. – Interminable Jul 05 '16 at 15:03
  • @Interminable - the point is that the only way to guarantee you are checking that the actual value is set is to compare to the value (rather than > 0). What happens if another developer comes along in 2 years time and adds a new enum? – Matt Wilko Jul 05 '16 at 15:16
  • @MattWilko Why would the addition of a new enum value affect anything? In testing adding extra flags in the example above, neither `If test And Names.Test2` (which won't work with `Option Strict On` anyway) nor `CBool(test And Names.Test2)` seem affected by the addition of other values to either the `Enum` itself nor the collection of flags. Unless I'm misunderstanding what's being said here? I should add that going for the comparison of the exact value is what I feel is the best way forward, based on your answer, but I don't see why the above would have any impact on either method. – Interminable Jul 05 '16 at 15:35
  • @MattWilko On the contrary, surely if the flag that's been added is a combination of flags including the one I'm checking for, why on Earth would I want for my check for the existence of that individual flag to not return true when that flag is present in the flag that is a combination of flags? – Interminable Jul 05 '16 at 15:54
  • @Interminable - I take your point, but it's also possible that an existing flag value changes in the future. The only way to *guarantee* you are checking the correct thing is to compare to the flag – Matt Wilko Jul 05 '16 at 16:00
  • @MattWilko Which is what I plan to be doing anyway, as I said earlier, following on from your answer. :P – Interminable Jul 06 '16 at 15:26