Private value in C# flags enumeration

Question

I'm creating a flags enumeration in C#, similar to the following:

    [Flags]
    public enum DriversLicenseFlags
    {
        None = 0,

        Suspended = 1 << 1,
        Revoked   = 1 << 2,

        Restored  = 1 << 3,
        SuspendedAndRestored = Suspended | Restored,
        RevokedAndRestored   = Revoked   | Restored,
    }

A couple notes about my intentions here:

• Suspended and Revoked are unique states that can, but don't necessarily lead to a restore.
• Restored should only be possible if the user has also been Suspended or Revoked (or both). It's important to track specifically which event was the precursor to the restore.
• A license can be both Suspended and Revoked (and Restored)

Also, I'm trying to stick to suggestions made in MSDN's Designing Flags Enumerations. In particular:

• Consider providing special enumeration values for commonly used combinations of flags.
  • SuspendedAndRestored and RevokedAndRestored will both be common.
• Avoid creating flags enumerations when certain combinations of values are invalid.
  • This is my problem, because Restored is not valid unless at least one of Suspended and Revoked is set.

Ideally, I'd like a value for Restored to be present in the enum for internal usage, but only available to set publicly via some valid combination. Unfortunately, internal isn't a valid modifier for enum values.

I've thought of a few alternatives, but each seems to have drawbacks:

1. Keep Restored as a public value, note the limitations in comments and do a precondition check for invalid combinations on public APIs.

   This would work, and is likely the solution I will go with. However, it seems like their should be a cleaner solution.

2. Use an enhanced, java-like enum as described here and define Restored as internal static.

   This would also work, but feels like overkill because I don't need any of the other functionality at this point.

3. Don't define Restored as a value, but reserve the value for OR'ing, and for checking the value in consuming methods. i.e.:

   internal const int RestoredFlag = 1 << 3;
   [Flags]
   public enum DriversLicenseFlags
   {
       None = 0,
   
       Suspended = 1 << 1,
       Revoked   = 1 << 2,
   
       SuspendedAndRestored = Suspended | RestoredFlag,
       RevokedAndRestored   = Revoked   | RestoredFlag,
   }
   

This feels hacky me, both in how it's defined and how it will be used internally.

Lise

Answer 1

Why do you want to use Flags specifically? Is this not exactly what you are after?

public enum DriversLicenseStatus
{
    None = 0,
    Suspended,
    Revoked,
    SuspendedAndRestored,
    RevokedAndRestored,
}

It will certainly be easy to make sure that only "valid" transitions are made from within the property setters of the DriversLicenseStatus properties.

If for some reason you definitely want to use Flags internally, then you can define a separate private enum DriversLicenseStatusFlags and convert from that to only expose DriversLicenseStatus values in your public interface.

Another option which could be worth considering is splitting the enum into two values:

bool IsActive;

enum InactiveReason {
    None = 0,
    Suspended,
    Revoked,
}

The "AndRestored" cases would be those with IsActive == true and InactiveReason != InactiveReason.None.

I get the distinct impression that you are over-engineering here. :)

Incase

You forgot SuspendedAndRevoked an SuspendedAndRevokedAndRestored. – Ormiston

@Jan: I don't think SuspendedAndRevoked makes sense. If it has been revoked, that's that. – Incase

It's what he asks for: Restored should only be possible if the user has also been Suspended or Revoked (or both) – Ormiston

@Jan: I think you are misreading the OP's intent. Notice that he has not defined those values either. – Incase

Good point. In my situation, it's also valid to be both Suspended and Revoked (and Restored). In that situation it would likely be more confusing if Restored isn't exposed. – Lise

@ScottWegner, @Jan: My bad then :) – Incase

@Jon, I like the idea of splitting it up into two separate variables for some "active" status and inactive reason. re: over-engineering, yes absolutely :) – Lise

Answer 2

Option 3 seems as good an idea as anything to me. Bear in mind that .NET enums will not throw exceptions when they are set to undefined values, so there is nothing preventing your users from saying:

PrintDriversLicenseStatus(42);

... where PrintDriversLicenseStatus takes a DriversLicenseFlags as its argument.

So you'll need to have each method check its inputs anyway to make sure they're a defined value.

Edit

One other option for your consideration: Create a special internal enum class, and cast arguments to it for internal use:

[Flags]
public enum DriversLicenseFlagsInternal
{
    None = 0,

    Suspended = 1 << 1,
    Revoked   = 1 << 2,

    Restored  = 1 << 3,
    SuspendedAndRestored = Suspended | Restored,
    RevokedAndRestored   = Revoked   | Restored,
}

[Flags]
internal enum DriversLicenseFlags
{
    None = DriversLicenseFlagsInternal.None,

    Suspended = DriversLicenseFlagsInternal.Suspended,
    Revoked   = DriversLicenseFlagsInternal.Revoked,

    SuspendedAndRestored = DriversLicenseFlagsInternal.SuspendedAndRestored,
    RevokedAndRestored   = DriversLicenseFlagsInternal.RevokedAndRestored,

}


public void DoSomething(DriversLicenseFlags arg)
{
    var argAsInternal = (DriversLicenseFlagsInternal) arg;
// or var argAsInternal = Util.CheckDefinedDriversLicense(arg);
}

Not sure if this is any better, but it may feel less hacky.

Varices

Sure, I can't guarantee that my users won't explicitly do strange/malicious things. However, I hope my design will discourage it. :) – Lise

@Scott Wegner: ... which is why I think option 3 is the best. It effectively presents users with only the logical options, without going to extremes. Create some internal utility methods to help check whether the drivers license is restored, etc., so the hackiness will all reside in a single class. – Varices

Answer 3

I would go for flags and create a validate method

[Flags]
public enum DriversLicenseFlags {
    None,
    Suspended,
    Revoked,
    Restored
}

public bool Validate(DriversLicenseFlags flags) {
    if(flags.HasFlag(DriversLicenseFlags.Restored)) {
        return 
            flags.HasFlag(DriversLicenseFlags.Revoked) || 
            flags.HasFlag(DriversLicenseFlags.Suspended);

        // Or throw an exception
     }
     return true;
}

If you can live with it being against the Microsoft recommendation. I see it as an edge case.

Ormiston