1

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?

  • Yeah, this is horrid. The only way it might make sense is if you encode lower and upper bounds using explicit syntax (i.e. enum item lower bound equals some specific value), but even then... – Robert Harvey Mar 16 at 23:14
4

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.

  • That makes sense. Would you say that if there's an agreement amongst all the developers working on a codebase to only use enums in this way, that you could do away with the bounds checks entirely? – chanban Mar 16 at 18:04
  • 1
    "Enums can only be out of range if you do arithmetic on them" - C# allows you to cast any arbitrary integer to any enum. – Sebastian Redl Mar 16 at 18:52
  • 1
    That is 'doing arithmethics' for me. You wouldn't assume that casting an integer value results in an animal, unless you're doing exactly what I described you shouldn't do. Enums should have no relation with ints. – Aganju Mar 16 at 19:01
  • 1
    Note that binary arithmetic with Flag Enums is perfectly legitimate. – Robert Harvey Mar 17 at 0:22
  • 1
    @Flater - Wouldn't just a set of constants make more sense than an enum in that case? – Errorsatz Mar 18 at 20:03
3

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.

1

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).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.