When are enums not a code smell?

Modern enums in languages with powerful compilers are one of my favourite features and I repeatedly find them to be a marker of high quality codebases.

In Python there has historically been a strong tendency to use strings as enums, which combined with Python's generally weak typing, can lead to all sorts of issues.

I also write a fair bit of Swift at the moment, and in Swift codebases I've found that plenty of use of enums, particularly those with attached values, leads to easy to use and correct code, as well as quite concise code. It's much better than codebases that go hard on classes.

Never knew enums are even considered to be code smell

Linters can/should already detect when a switch on enum doesn't handle all cases

Did you link to this SE post so we could laugh at it?

Never smells: You are using a language with discriminated unions (before or after 90s)

Smells: You are using a dynamically typed language, and you used a hack to implement them

Sometimes smells: You are using a 90s language with a horrible implementation of enums

Smells: You are an OOP charlatan and they do not fit your invalid world view and your enums have methods on them plus some other bezerk shit. plus you have cognitive dissonance because you were told objects are expensive and you should really only pass ints around willy nilly and everything else should be god objects

In a functional language like Haskell or O'Caml, it feels like half of what you write involves enums (sum types). They're fantastic in those contexts and I really miss them in python. But OO is a different approach to data and I suspect the "code smell" advice comes from not wanting to mix the two paradigms. For example

Functional:

    data Switch = On | Off
    function flip: Switch -> Switch
    flip s = case s of
       On  -> Off
       Off -> On

Object Oriented:

    abstract class Switch { Switch flip(Switch); }
    class On : Switch  { flip(s) { return new Off(); }
    class Off : Switch { flip(s) { return new On(); }

Weird mixture:

    enum State {On, Off};
    class Switch {
        State myState;
        Switch flip(Switch s) {
           if (s.myState == Off) { s.myState = On;  return s ; }
           if (s.myState == On)  { s.myState = Off; return s; }

Design patterns are often heavily weighted towards the languages in which they were devised and don't generally apply.

Lots of the classic OOP design patterns are because Java/C++ is very rubbish.

I'm going to flag this submission. This is horrible Stack Overflow post with bad answers.

Languages have different types of "enums". Enums in languages like Rust are extraordinarily powerful sum types that create compact memory footprints, strong compile time guarantees, and can form the basis of expressive and correct state machines.

Enums (sum types) are one of the best patterns.

This is a great example of why the "clean code" movement is so destructive. Writers (bloggers, textbook authors, professors, whatever) can very easily take advantage of inexperienced developers who are dying for someone to tell them what is and isn't the right way to do things, and virtually all "code smell" style blanket rules are detrimental to learning. In this case and in many other cases, the rules doesn't tie back in any way to what the code actually needs to do.

For me, enums are just a way to eliminate canonical magic strings throughout. They also have the happy side effect of storing as an integer in the database, but I do sometimes apply a .ToString() in those situations where users will be looking at the data frequently.

There are actually very few cases where I would say enums should not be used. The most obvious one being open-ended types where the list of members will always be growing (e.g. if you made each user of a system a member). You should also not use enums to convey a compounding of logical facts - these get nasty really fast in my experience. Sometimes it is better to just have 5-10 boolean flags on a type than to try and cram every logical combination of those things into canonical enumeration members.

I really hope that 'enums are a code smell' is the worst technical opinion I see today because I'm not sure I could take much worse.

Generally the enum vs polymorphic class debate is actually a subset of a better known trade-off between OO style inheritance and FP style use of sum types.

The trade-off is between ease of extension in two cases: - adding new types to the sum type / new subclasses - adding new methods that can operate on all types / subclasses

In a classic OO design, it's easy for any client to add new subclasses that implement the same API as the existing classes. However, adding a new API means that all subclasses need to be changed.

Conversely, in an FP design, adding a new type to the sum type (a new enum value) means that all places pattern matching on the sum types (switching on the enum) need to be changed. However, adding a new operation that works with the existing types is trivial.

To give a more specific example, let's say we want to model an expression tree.

An OO design might look like this:

  abstract class BaseExpression {
    abstract evaluate()
    subexpressions()
  }
  class SumExpression extends BaseExpression {
    evaluate() = map(evaluate, subexpressions()).Sum()
  }
  class NumberExpression extends BaseExpression {
    int x
    subexpressions() = [x]
    evaluate() = x
  }

An FP design might look like

  type ExpressionTree a = SumExpresssion [ExpressionTree] | NumberExpression a

  evaluate :: ExpressionTree a -> a
  evaluate SumExpression xs = sum map evaluate xs
  evaluate NumberExpression x = x

  subexpressions :: ExpressionTree a -> [a]
  subexpressions SumExpression xs = map union xs
  subexpressions NumberExpression x = [x]

Equivalently, an enum based design would have

  enum ExprType {
    Sum
    Number
  }
  
  struct SumExpression {
    ExprType
    *void[] Subexpressions
  }

  struct NumberExpression {
    ExprType
    int x
  }
  
  evaluate(e) {
    switch(e->ExprType) {
      case Sum: (SumExpression)e.Subexpressions.map(evaluate).Sum()
      case Number: (NumberExpression)e.x
    }
  }

  subexpressions(e) {    
    switch(e->ExprType) {
      case Sum:  return (SumExpression)e.subexpressions
      case Number: void*[]{(NumberExpression)e.x}
    }
  }
  

Now, if we want to add support for MultiplyExpression, in the OO program we add a new file and we define evaluate() for MultiplyExpression. In the FP program we have to modify the definitions of ExpressionTree, SumExpression and NumberExpression.

Conversely, if we want to add support to print an ExpressionTree, in the FP version we just add a new function in a new file and describe how to print each type of ExpressionTree. In the OO program, we need to modify the implementations of each existing BaseExpression subclass to add support for printing.

In C#, enums basically add typing to a predefined set of entities or statuses. This is why they basically are just numbers under the hood.

Practically, though, they are important when dealing with serialization: Writing state to disk, database, or over the wire. Because they quickly typecast to a number or a string, you don't have to deal with the complexity of serializing or deserialzing.

Having recently inherited a C# codebase where enums weren't used, what I can say is that "polymorphic classes" to represent a predefined set of things is a smell. This manifests itself in cases where we had tables with a limited set of constants, and were loading them through joins. It was much more efficient to remove the join and turn the foreign key into an enum. In another case, the "object" is fully serialized in a contract to JSON. It's much more efficient to just serialize a single value in the contract as well.

The key IMO is mentioned in one of the comments.

> [lots of] switches on enums are a code smell

Someone else also pointed out that a code smell doesn't mean something is always bad - only that it's likely to be bad. An enum that's rarely or never the subject of an if/switch check or pattern match is probably OK. For example...

* A type-safe and self-describing alternative to a boolean argument

* A scheduler priority or group

* A state in a state machine

OTOH, an enum that appears often in conditionals is likely to be error prone and classes or interfaces should be considered instead. But it's really all pretty context-dependent. The question is: which approach creates more coupling and/or requires more points in the code to be updated to remain correct when a new value is added?

Enums aren’t a code smell.

I'm in the minority I guess on not liking enums. The main thing I've run into repeatedly is difficulty to add to them without breaking things. Even if linters catch non-exhaustive switch case statements, there's always the if statement that it misses. Client compatibility is a big challenge, so I usually end up sending them as strings on the wire anyway.

It is also good to remember that different programming languages have different features for enums. e.g. in some languages you cannot print the enum as string, and in some languages you cannot choose the underlying type of the enum.

For defining bitmasks and the like in (embedded) C I'm mostly using enums instead of preprocessor macros. Looks better in syntax highlighting and works better in autocompletion.

I don't have a SE account so I'm going to provide the answer he's searching for to get out of the switch statement: dynamic object construction.

He's got code that looks like this:

  switch(type) {
    case BidPrice:
    case BidSize:
      MatchSellOrders();

The solution to ditching the switch statement is to create small classes whose names and locations conform to a convention so they can be inferred from the type value:

  class BidPriceOrders : MatchSellOrders {}
  class BidSizeOrders : MatchSellOrders {}

The corollary to switch statements is "primitive obsession" and that's the name for the code smell that includes enums. An enum doesn't have behaviour. It's just primitive data with an expressive symbol and namespace. The switch exists to couple behaviour to the symbol, both of which exist independently of each other because of poor design. The basic idea of OOP is all about grouping data with the methods that operate on it so that they live and evolve together as objects.

A switch is a middleman that has to understand the type and the behaviour as things that live and evolve separately. It's a point of coupling that needs to be maintained independently of the data and methods. In other words: it's superfluous code; unnecessary complexity that any code-base would benefit from not having to deal with.

So the idea is to replace the enum with an object without having to introduce the switch. He's right that you need a factory, but he's wrong that you need a switch statement to implement the factory. You can also just stick a static "factory method" on the abstract base class that defines the generic object type to keep the code / overhead as low as possible. The pseudo-code implementation is:

  abstract class BidType {
    static create(typeFlag: BigTypeFlag): BidType {
      const className = `${BidTypeClassPath}/${typeFlag}Orders`;
      if(!notExists(className)) {
         throw new IllegalArgumentException('Unsupported bid type');
      }
      return new className();
    }
  }

  const bidType = BigType::create(typeFlag);

Every language will have a different mechanism that allows you to dynamically instantiate an object but that's the basic idea.

The only time they ARE a code smell is when the values are only used one place and they could have been in-lined for readability.

it's much better than dogmatically attempt to model things in OOP, fail to do that, then resort to "instance of" in god knows where in the codebase.

> "WARNING As a rule of thumb, enums are code smells and should be refactored to polymorphic classes. [8]" Seemann, Mark, Dependency Injection in .Net, 2011, p. 342

Gosh, what a load of baloney from a frog in a well.