Use of Visitor pattern rather than enums

Question

Lets say that a enum exists:

public enum SomeEnum
{
    FirstValue,
    SecondValue
}

It's use is spread through the code and all sort of wonderful logic hangs off it. If we now add another entry (ThirdValue?), we'd have to find all the places in the code that make use of it and update.

Either that or run the risk of defaults throwing exceptions during runtime (if default just throws an exception), or just some cases aren't handled.

This applies even more so if it is a library that has been used and any updates to enum values hasn't been documented/reviewed.

If we replaced it with a simple visitor/double-dispatch pattern (condensed for simplicity):

public interface ISomeEnumVisitor
{
    void VisitFirstValue();
    void VisitSecondValue();
}

public interface IEnumValue
{
    void Accept( ISomeEnumVisitor visitor );
}

internal class FirstValue : ISomeEnumVisitor
{
    public void Accept( ISomeEnumVisitor visitor )
    {
        visitor.VisitFirstValue();
    }
}

internal class SecondValue : ISomeEnumVisitor
{
    public void Accept( ISomeEnumVisitor visitor )
    {
        visitor.VisitSecondValue();
    }

}

public static class EnumValues
{
    public static FirstValue First { get; } = new FirstValue();
    public static SecondValue Second {get; } = new SecondValue();
}

Adding a ThirdValue class to the ISomeEnumVisitor would cause a compile-time error if not implemented.

Surely this is better even though it is (obviously) much more verbose.

Is this a valid thing to be concerned about?

Answer 1

Yes, the visitor pattern can be used as an alternative to enums+switch/case. I would consider this if your enum variants are more complicated (need to carry associated data, not just represent an integer), or if it is very important that all variants are always considered.

Limitations of C# enums

In C#, enums just provide names for some values of the underlying integral type, but should not be considered to be exhaustive:

The set of values of the enum type is the same as the set of values of the underlying type and is not restricted to the values of the named constants.

Thus, when switching on an enum, it is necessary to have a default case (or a discard pattern in case of a switch-expression). It is not reasonably possible to exhaustively handle all possible values of the enum.

Consequences of using class hierarchies instead

Indeed, the alternative is to use a class hierarchy to define the alternatives. By making information about the variants accessible to the type system, we can get an error when we forgot to handle a case. However, there is some mismatch between this enum-oriented thinking and OOP approaches.

There are two ways in which such a class hierarchy could be used: using the visitor pattern, or adding methods to an interface. Or possibly, a combination of both.

You already mentioned the visitor pattern. Implementing it does require a bit of extra boilerplate, but it's not so bad. The visitor pattern makes it easy to create more visitor implementations, i.e. to create new operations that process the entire class hierarchy. However, adding a new variant (introducing a new class) is a breaking change, requiring all existing visitors to be updated. This is exactly what you want. But this also means that this kind of change tends to be inappropriate for the API of a library, as changing this API would break all downstream code.

In some cases it might be possible to provide a fallback implementation for an easier migration (e.g. by providing a visitor base class with stub implementation), but this would prevent the compiler from alerting consumers that a new option was added, leading us back to the problem with C# enums…

C# does offer convenient syntax for checking the type of a value with switch-expressions, but there's no information to tell the type checker what all possible subtypes are, preventing exhaustiveness checking.

An alternative is to just use a normal OOP design, with an interface or base class that declares all possible operations. Each variant is a class that then implements these methods. This makes it easy to introduce new variants, but adding a new operation to the interface or base class would be a breaking change that requires all variants to be updated.

Note that these techniques – declaring variants in a visitor interface vs declaring operations in a base class – both allow one kind of extensibility while making extensibility along another axis difficult: you can either make it easy to add new operations or new variants, but not both.

Another significant consequence of using your own classes to define variants is that you're no longer limited to just having integers as the underlying data – your variants can include arbitrary fields. Whereas a C# enum is a collection of named integers, these type hierarchies define a collection of classes that may in turn have multiple distinct instances. In the visitor pattern, all instances of a class are handled as one case, which may or may not be desirable.

How other languages do it

Other languages have direct support for exhaustiveness checking, making it unnecessary for workarounds like the visitor pattern. Many functional languages have "sum types". E.g. Rust has enums that don't just name some integers, but that define a type that can only have the given values. This lets the compiler show an error when one variant was forgotten, though it's also possible to opt-in to non-exhaustive enums that always require a fallback case (great for evolving APIs in a backwards-compatible manner).

C++ has std::variant to construct enum-like types in an ad-hoc manner. Instead of having the variants inherit from a common interface, the variants are defined first and then combined. Then, std::visit can be used to match on the values in the variant group. This uses C++ features like templates, compile-time conditionals, and static_assert to ensure exhaustiveness at compile time. It's better than writing visitors by hand, but it's not necessarily pretty either.

Some languages have built-in support for defining closed class hierarchies that then allow exhaustiveness checking. E.g. Kotlin lets us define a "sealed" class or interface that we can only inherit in the current file. This allows the compiler to know when a when expression (Kotlin's switch-expression) has exhaustively matched all possible types. I think it would be delightful if C# got this feature as well.

Java inhabits a weird spot in this design space. Its enums are conceptually a class, and variants name all valid instances of this class. You can't cast arbitrary ints to an enum. Thus, exhaustiveness checking would be possible. However, Java has compatibility rules that say that adding a new enum member is not a breaking change, thus the compiler can at most emit a warning when a case was missed.

Answer 2

Is there something else happening in your code that hasn't been disclosed? Additive changes to enums shouldn't result in failures elsewhere in the app unless that code is using enums inappropriately.

If the enum is being used in a switch statement, that switch should have a default operation for all non-matching cases, so if the new value is encountered, that default path should still succeed.

If the enum is being used for a comparison operation, the comparison operation should also still succeed despite the new value so long as you're not using any underlying representation of the enums and switching values (ala https://dougseven.com/2014/04/17/knightmare-a-devops-cautionary-tale/).

Wrapping enum evaluation inside a double dispatched visitor pattern is an unnecessary layer of indirection that is likely to confuse future developers without providing any guards against failure. Enums are typed representations of static values that (should) have no state or inherent functionality. They should only be considered a convenience mechanism that allows you to avoid having to reference explicit numerical values or strings.

Answer 3

It seems to me that that's an odd implementation of the visitor pattern.

You want to be able to write new visitors that ignore, or group element types, you don't want the element type holding logic which should be in the visitor. ie should I handle this element as a type A or a type B

To have this work you need a stable interface for the visitor classes which can be reused for lots of new operations. Indeed I would expect the visitors method to be testing the type in exactly the kind of switch case you want to avoid, and only operating on selected types while ignoring the rest.

I think a better solution to the problem you present, would be a linter. It should be fairly easy to detect conditionals where the enum of used, check that all possible values of the enum are handled and throw an error if not.

https://www.jetbrains.com/help/rider/Reference__Code_Inspections_CSHARP.html#BestPractice

Also note that c# does throw warnings for non exhaustive switch expressions

https://github.com/dotnet/csharplang/blob/main/proposals/csharp-8.0/patterns.md#switch-expression