tags:

views:

5346

answers:

5

Hello, I am building a fun little app to determine if I should bike to work.

I would like to test to see if it is either Raining or Thunderstorm(ing).

public enum WeatherType : byte
{ Sunny = 0, Cloudy = 1, Thunderstorm = 2, Raining = 4, Snowing = 8, MostlyCloudy = 16 }

I was thinking I could do something like:

WeatherType _badWeatherTypes = WeatherType.Thunderstorm | WeatherType.Raining;
if(currentWeather.Type == _badWeatherTypes)
{
 return false;//don't bike
}

but this doesn't work because _badWeatherTypes is a combination of both types. I would like to keep them separated out because this is supposed to be a learning experience and having it separate may be useful in other situations (IE, Invoice not paid reason's etc...).

I would also rather not do: (this would remove the ability to be configured for multiple people)

if(WeatherType.Thunderstorm)
{
 return false; //don't bike
}
etc...
+1  A: 

use the FlagsAttribute. That will allow you to use the enum as a bit mask.

MagicKat
You can use the enum as a bitmask without flags; FlagsAttribute just changes how ToString operates, I believe. It would still be a good idea to include it, admittedly.
Jon Skeet
It does just change how ToString behaves, but there is no guarantee that future versions of .NET won't change other behaviors (like the compiler issuing errors if the values aren't powers of 2) so you should always use it when using a flags enum.
Scott Dorman
Ah, I didn't know that it just changed the ToString(). Interesting.
MagicKat
If you don't specify the values, Flag will set them appropiately to powers of 2 (at least in VB).
Mark Brackett
Flags doesn't change the autogenerated values in C#, unfortunately.As for "no guarantee that future versions of .NET won't change other behaviours" - that's like saying "there's no guarantee that the garbage collector will still work in future versions, better set variables to null to help it."
Jon Skeet
To say more about my last comment - this would require the C# team to break section 7.3.2 of the spec, for no good reason. Yes, using Flags is a good idea. No, it's really, really not necessary.
Jon Skeet
I believe FlagsAttribute also changes how enum.Parse() works.
James Curran
Hmmm...looks like my last comment didn't make it...Jon is correct, using Flags doesn't cause the compiler to autogenerate proper power of 2 values, even in VB. (I just checked this.)
Scott Dorman
James - you're right, sorry, I forgot that side of things. Good catch.
Jon Skeet
@Jon Skeet, my comment about no guarantees about behavior was actually arguing _for_ using the Flags attribute. Just because you _can_ use an enum as if it were a flags enum without the attribute doesn't mean you should. There is no guarantee that those scenarios will always work in the future.
Scott Dorman
By using the Flags attribute you explicitly tell the compiler and the runtime that you expect a certain behavior from the enum.
Scott Dorman
Scott: Yes, I know you were arguing for it. And I agree it's a good idea. I just don't believe you really need it at all, and the "they might change the spec" argument is flawed, as you can apply it to anything else. They might redefine addition and subtraction - how do you guard against that?
Jon Skeet
That's "don't really need it" for the bitwise side - for ToString and Parse it's necessary to get the right behaviour, and it's also good in terms of documenting expectations.
Jon Skeet
Just looked through the spec, and I can't see any reference to the Flags attribute at all, so I don't believe it's affecting the compiler - just the framework.
Jon Skeet
Looking at the code for Enum.Parse, it doesn't look like it takes the Flags attribute into account. If it gets a comma-separated list it will bitwise-or them together no matter what.
Scott Dorman
Correct, right now it just affects the runtime. (Would be nice if the compiler validated the enum values, however.) In that sense, you are also correct in that you "don't really need it", but it does explicitly tell anyone who's looking how the enum should behave. There is no reason not to use it.
Scott Dorman
It was added to the runtime for a reason, specifically to indicate that an enumeration can be treated as a bit field. There are certain things you can't guard against being redefined (as in your examples) but there are things you can do to mitigate the results.
Scott Dorman
Yes, it was added for a reason - but are you really going to suggest there's more than a million to one chance that the C# team will break the spec (which defines the behaviour very precisely) for no reason, and break potentially many apps?
Jon Skeet
And yes, I agree there's no reason not to use it. I was *only* arguing against the "it might break in the future" logic :) Thanks for the catch about Parse, btw. Fixed my expanded explanation.
Jon Skeet
No, I'm not suggesting that at all (although it happened moving from 1.1 -> 2.0). Anyway, the more likely scenario is that the compiler gains some knowledge about them and is able to do more compile time checks.
Scott Dorman
Wow - I was completely wrong. It seems that Flags *does not* autogenerate the proper values for you in either VB or C# - nor will it complain if you then bitwise them. I'm not sure why I thought that was the case. Excuse me while I scurry off to look for any enum bugs over the last 5 years!
Mark Brackett
+12  A: 

Your current code will say whether it's exactly "raining and thundery". To find out whether it's "raining and thundery and possibly something else" you need:

if ((currentWeather.Type & _badWeatherTypes) == _badWeatherTypes)

To find out whether it's "raining or thundery, and possibly something else" you need:

if ((currentWeather.Type & _badWeatherTypes) != 0)

EDIT (for completeness):

It would be good to use the FlagsAttribute, i.e. decorate the type with [Flags]. This is not necessary for the sake of this bitwise logic, but affects how ToString() behaves. The C# compiler ignores this attribute (at least at the moment; the C# 3.0 spec doesn't mention it) but it's generally a good idea for enums which are effectively flags, and it documents the intended use of the type. At the same time, the convention is that when you use flags, you pluralise the enum name - so you'd change it to WeatherTypes (because any actual value is effectively 0 or more weather types).

It would also be worth thinking about what "Sunny" really means. It's currently got a value of 0, which means it's the absence of everything else; you couldn't have it sunny and raining at the same time (which is physically possible, of course). Please don't write code to prohibit rainbows! ;) On the other hand, if in your real use case you genuinely want a value which means "the absence of all other values" then you're fine.

Jon Skeet
[Flags] does seem like the way to go here.
Mark Bessey
Well, it's definitely a good thing to do - but it's entirely orthogonal to the actual question. It will neither help nor hinder the bitwise logic.
Jon Skeet
You were correct about "Sunny" I needed to change it. Thanks for all the help everyone.
Nathan Koop
A: 

You need to use the [Flags] attribute (check here) on your enum; then you can use bitwise and to check for individual matches.

David Alpert
Please see my response to MagicKat. Flags is desirable but not necessary here.
Jon Skeet
A: 

You should be using the Flags attribute on your enum. Beyond that, you also need to test to see if a particular flag is set by:

(currentWeather.Type & WeatherType.Thunderstorm == WeatherType.Thunderstorm)

This will test if currentWeather.Type has the WeatherType.Thunderstorm flag set.

Scott Dorman
A: 

I wouldn't limit yourself to the bit world. Enums and bitwise operators are, as you found out, not the same thing. If you want to solve this using bitwise operators, I'd stick to just them, i.e. don't bother with enums. However, I'd something like the following:

  WeatherType[] badWeatherTypes = new WeatherType[]
  { 
      WeatherType.Thunderstorm, 
      WeatherType.Raining
  };

  if (Array.IndexOf(badWeatherTypes, currentWeather.Type) >= 0)
  {
                        return false;
  }
Ken Wootton
This is exactly what flags enums (bit fields) are designed to do. From the MSDN page on the FlagsAttribute: Bit fields are generally used for lists of elements that might occur in combination...Therefore, bit fields are designed to be combined with a bitwise OR operation to generate unnamed values.
Scott Dorman