How to optimize enum assignment in C#

Question

I have got this enum

enum NetopScriptGeneratingCases
{
    AddLogMessages,
    AddLogErrors,
    AddLogJournal,
    AllLog = AddLogMessages | AddLogErrors | AddLogJournal,
    DoNothing
}

And there is UI with 3 checkboxes so depending which of them are checked I have to generate possible cases to do some job.

NetopScriptGeneratingCases netopScriptGeneratingCases = NetopScriptGeneratingCases.DoNothing;

if (checkBoxAddAuditLog.Checked)
{
    netopScriptGeneratingCases = NetopScriptGeneratingCases.AddLogJournal;
}
else if (checkBoxAddErrorLog.Checked)
{
    netopScriptGeneratingCases = NetopScriptGeneratingCases.AddLogErrors;
}
else if (checkBoxAddLogMessages.Checked)
{
    netopScriptGeneratingCases = NetopScriptGeneratingCases.AddLogMessages;
}
else if (checkBoxAddAuditLog.Checked || checkBoxAddErrorLog.Checked)
{
    netopScriptGeneratingCases = NetopScriptGeneratingCases.AddLogJournal | NetopScriptGeneratingCases.AddLogErrors;
}
else if (checkBoxAddAuditLog.Checked || checkBoxAddLogMessages.Checked)
{
    netopScriptGeneratingCases = NetopScriptGeneratingCases.AddLogJournal | NetopScriptGeneratingCases.AddLogMessages;
}
else if (checkBoxAddErrorLog.Checked || checkBoxAddLogMessages.Checked)
{
    netopScriptGeneratingCases = NetopScriptGeneratingCases.AddLogErrors | NetopScriptGeneratingCases.AddLogMessages;
}
else if (checkBoxAddErrorLog.Checked || checkBoxAddLogMessages.Checked || checkBoxAddAuditLog.Checked)
{
    netopScriptGeneratingCases = NetopScriptGeneratingCases.AddLogErrors | NetopScriptGeneratingCases.AddLogMessages | NetopScriptGeneratingCases.AddLogJournal;
}


var modifiedFiles = NetopScriptGenerator.GenerateNetopScript(netopScriptGeneratingCases, netopFiles);

But I am not sure that this code is a correct... Are there other ways to do it?

Parsaye

If you're going to use your enum as a set of flags as you do by or'ing them together to create AllLog then you need to give them values that are powers of 2. That is you should set the first one to 1, then 2, then 4, and DoNothing should be set to 0. – Goalkeeper

The way the code is written is flawed I think, since the first check will swallow a lot of other checks; you should try with the most specific and let more general cases to the end. – Slavish

Answer 1

I would take what Chris suggest in his answer and assign your variable like so:

NetopScriptGeneratingCases netopScriptGeneratingCases = NetopScriptGeneratingCases.DoNothing;

if (checkBoxAddAuditLog.Checked)
{
    netopScriptGeneratingCases |= NetopScriptGeneratingCases.AddLogJournal;
}
if (checkBoxAddErrorLog.Checked)
{
    netopScriptGeneratingCases |= NetopScriptGeneratingCases.AddLogErrors;
}
if (checkBoxAddLogMessages.Checked)
{
    netopScriptGeneratingCases |= NetopScriptGeneratingCases.AddLogMessages;
}

Videogenic

Answer 2

As mentioned in the comment, the compiler will by default give enums an incrementing integer value, which isn't suitable for using as bitflags. Try amending your definition as follows:

[Flags]
enum NetopScriptGeneratingCases
{
        DoNothing = 0,
        AddLogMessages = 1<<0,
        AddLogErrors = 1<<1,
        AddLogJournal = 1<<2,
        AllLog = AddLogMessages | AddLogErrors | AddLogJournal
}

Bianka

Addendum: to do the opposite, so check which are set, you AND against that field (e.g. if ( values & Enum.AddLogMessages )) or a bitmask of 1s to get them all. – Pediculosis