Using extraneous enum members for bounds/validity checking

Question

A coworker of mine insists that this is the right way to write enums in C#

public enum ExampleEnum
{
    InvalidItem,
    Item1,
    Item2,
    Item3,
    MaxItem
}

We have enums defined like this all over our codebase. Everywhere we use the enum we then check that the values are between InvalidItem and MaxItem.

To me this seems unnecessary since you can check Enum.IsDefined and Enum.GetValues().Length to get the same information.

I'm not a software engineer but recently starting to fill the role for the company I work for. What is the best practice here?

Answer 1

Enums can only be out of range if you do arithmetic on them - adding 1 to get to the next one, etc.
This is the root of any problems you could encounter with out-of-bounds.

The solution is of course to not allow any arithmetic on enums. Treat them as independent, non-consecutive values, and you won't need these bookend-values.

Example: Cat, Dog, Mouse. You would never get the idea to code Dog = Cat + 1.

Answer 2

While the underlying type of an enum is an integer (or other discrete numeric type, like long), there is not much use in treating them as numbers, making bounds-checking a needless operation. As mentioned in Aganju's answer, it makes no sense to think of Dog = Cat + 1, or Blue = Yellow - 2. In your example, Item1 and Item2 are related conceptually by whatever commonality they have (return status, shape, color, animal, vehicle type, whatever), but there is no inherent mathematical relation between them.

One time it might come into play is when (de)serializing and you're not using the existing methods to convert your serialized value (e.g., a string) back to an enum. The solution there is not to do bounds-checking, but to use the proper method to do the conversion (i.e., Enum.TryParse<ExampleEnum>(serializedValue, out ExampleEnum enumValue) ). One other case may be if you're interfacing with some (external) non-C# libraries, but even then I fail to see the need for a bounds-check.

Bottom line is that I'd be suspect of situations where your bounds-check fails. Any proper (semi-modern) C# code should not get into that situation, and if it does, you're probably using the wrong type. For example, an enum when you should be using an int or an int when you should be using an enum.

Answer 3

Lots of things are good or bad depending on how they interact with your tools.

For example, the tools that I use really like it if a switch statement based on an enum covers all values explicitly. Which means I have to add a case for a non-existing maxItem to avoid warnings. Yes, you could use "default". But that means if I don't handle Item2 by mistake, and use "default" to handle all the undefined cases, I now have a bug that my tools cannot warn me about.

You could add FirstItem = Item1 and LastItem = Item3, so all you need to do is make sure the enum values are consecutive, and update LastItem if you add a further case. Your check for validity would be (case >= FirstItem && case <= LastItem). I think this is a bit better; you can now do a check that you have no value outside the range, and still rely on compiler warnings to find other mistakes.

You can remove the MaxItem if you switch to a language where illegal enum values are impossible. I don't know about C#; in Swift it is impossible to have an out of range enum value (except possible causing it with malicious intent, and even that is difficult).