4

I am trying to use a set of conditional statements that will set up an enumeration attributed with [Flags]. However, the compiler complains that 'm' is unassigned. How can I rewrite the following to achieve my intended functionality?

Media m;
if (filterOptions.ShowAudioFiles)
    m = m | Media.Audio;
if (filterOptions.ShowDocumentFiles)
    m = m | Media.Document;
if (filterOptions.ShowImageFiles)
    m = m | Media.Image;
if (filterOptions.ShowVideoFiles)
    m = m | Media.Video;
EricSchaefer
  • 25,272
  • 21
  • 67
  • 103
deckerdev
  • 2,766
  • 2
  • 21
  • 23

7 Answers7

18

You need to initialize m. Create a "None" flag that has value 0 then:

Media m = Media.None;

Then the rest of your code.

i_am_jorf
  • 53,608
  • 15
  • 131
  • 222
2

You could also write: 

Media m = default(Media)

Useful in cases where you don't know the enum, class, or whether it's a value/reference type.

lightw8
  • 3,262
  • 2
  • 25
  • 35
  • 1
    This is really a bad idea, it hides the whole simplicity of enumerations. They were created to remove magic numbers and you are adding it back in when there should be a None for a flags enum that you should use instead. – Samuel Apr 10 '09 at 20:33
  • Good point. I was thinking about the case where you aren't the author, but then explicit choice of the default would be better anyway. – lightw8 Apr 20 '09 at 17:47
1

If none of the conditions are true m will be undefined. Set it to an initial value.

EricSchaefer
  • 25,272
  • 21
  • 67
  • 103
1

Do you have a 'default' like filterOptions.ShowNone? If so, start off with m set to that. The compiler is complaining becuase at the end of all the if's, m might not be set to anything.

n8wrl
  • 19,439
  • 4
  • 63
  • 103
0

The Problem with the accepted approach is, that you need to have a default element. In case you do not want to have the None element, then you can use nullability.

Media? m = null;
if (filterOptions.ShowAudioFiles)
    m ??= Media.Audio;
    m = m | Media.Audio;
if (filterOptions.ShowDocumentFiles)
    m ??= Media.Document;
    m = m | Media.Document;
if (filterOptions.ShowImageFiles)
    m ??= Media.Image;
    m = m | Media.Image;
if (filterOptions.ShowVideoFiles)
    m ??= Media.Video;
    m = m | Media.Video;

// in case you require a value for m you can do:
if( m is null ) { throw ... }
Pascal Senn
  • 1,712
  • 11
  • 20
0

In addition to the above answers, besides the fact that this code seems pretty redundant, I'd like to suggest that you use a Select Case instead of all those ugly If's.

hmcclungiii
  • 1,765
  • 2
  • 16
  • 27
-5

You don't really need to create a Media.None. You can cast any value to the Flag enum even if it doesn't equal to a value of the flags.

Media m = (Media)0;

if (filterOptions.ShowAudioFiles)     m |= Media.Audio; 

if (filterOptions.ShowDocumentFiles)  m |= Media.Document; 

if (filterOptions.ShowImageFiles)     m |= Media.Image; 

if (filterOptions.ShowVideoFiles)     m |= Media.Video;
Wai Ha Lee
  • 8,598
  • 83
  • 57
  • 92
Matthew Whited
  • 22,160
  • 4
  • 52
  • 69
  • What if all the conditions fail? What if 0 is e.g. Media.Audio? You need a *valid* initial value. – EricSchaefer Apr 10 '09 at 20:06
  • This is bad advice. You are always better off using explicit values than relying on integer casts. – Randolpho Apr 10 '09 at 20:13
  • Not really. The enum will still work perfectly fine even if there isn't a "valid" flag set. There will still be a value. All enums are inherited from "Int32" by default. Yes there could be other problems later if you don't handle default values. But that is what exceptions are for. – Matthew Whited Apr 10 '09 at 20:15
  • Since it's a flags enum, the None value ought to map to 0 (no bits set). Doing it any other way would just be weird. On the other hand you may want a Media.Default which does have some bits set. – tvanfosson Apr 10 '09 at 20:17
  • Ouch. So you wait until later when an exception is raised rather than initializing your data properly in the forst place? Ouch again. – EricSchaefer Apr 10 '09 at 20:19
  • Keep in mind the Media.None can be confusing as well becasue you can't do a mask check for the values. If you have standard logic to check for an enum value such as if (input & selectedmask == selectedMask) will always true if selectedMask = Media.None – Matthew Whited Apr 10 '09 at 20:20
  • @Matthew: If someone were to use Media.None in that fashion, it's their own damn fault for not understanding enums. – Samuel Apr 10 '09 at 20:35
  • I'm enjoying that this is still collecting down votes even though `Media m = 0;` alone is valid c#. – Matthew Whited Sep 18 '19 at 15:49