Investigate: Why is ConsistencyMode a class instead of an enum?

[sleberknight]

The ConsistencyMode class looks very much like an enum, and even replicates the behavior of enums, e.g. it defines name(), ordinal(), and values() methods. Hopefully there is some clue in the git commit history.

[sleberknight]

ConsistencyMode was changed from an enum to a class in 6fb5d0b. The commit comments state:

I took the path to change the enum ConsistencyMode into a class emulating enum's
methods in order to keep backwards compatibility and avoiding breaking clients.

This commit was made in January 2020, and it is now August 2023. I was only using the library at that point when it was maintained by Rick Fast. It appears that the main reason it was changed from an enum to a class was to allow the "additional headers" as mentioned in the commit message:

This patch is a bit intrusive since I have to dynamically change Header Cache-Control
during requests and current static ways to inject headers using injectors was not
sufficient.

It is true that this would not be possible with an enum, but maybe this could have been done by introducing a separate enum class. The static factory method createCachedConsistencyWithMaxAgeAndStale suggests a possible design:

public class ConsistencyMode {

    private final String name;
    private final ConsulConsistencyMode consulConsistencyMode;
    private final String param;
    private final private final Map<String, String> additionalHeaders;

   // constructors and/or factory methods, builder, getters, etc.
}

public enum ConsulConsistencyMode {
    DEFAULT, STALE, CONSISTENT
}

In the above, ConsistencyMode remains a class but the “enum-y” part of the consistency mode is extracted into a separate enum class, ConsulConsistencyMode. The naming is somewhat unfortunate, but I could not come up with a good name for the enum that wasn’t just ConsistencyMode. It could just be Mode but I figured ConsulConsistencyMode is more descriptive. This design removes the need to basically replicate the behavior of Java enums, while retaining the ability to create new instances of ConsistencyMode with custom headers and param.

However, createCachedConsistencyWithMaxAgeAndStale creates a new instance that acts like a fourth enum value, sort of like if you could create new enum constants at runtime. This makes the above design not really work very well, for example what value would be used for the ConsulConsistencyMode?

Another possible (now that we’re on JDK 17) design might be to make ConsistencyMode a sealed class and permit subclasses that represent default, stale, consistent, or cached consistency modes. But reading the Agent Caching page on Consul’s developer site, the caching part isn’t really a consistency mode; rather, caching is orthogonal and may work with any of the consistency modes.

As a side note, all usages of the createCachedConsistencyWithMaxAgeAndStale factory method are in tests! There is no production code using it. In addition, the STALE and CONSISTENT constants in ConsistencyMode are also only used in tests; only the DEFAULT constant is used in production code, in KeyValueClient, QueryOptions, and TransactionOptions.

But assuming this stays as a class, does it really need to pretend to be like an enum? Or can the “enum-like” methods and behavior such as ordinal()and values() be removed? As mentioned above, maybe a sealed class hierarchy would work well here.