7 ms·
> "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.
by fivelessminutes 5y ago
> "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.
- kaetemi 5y agoThat guy must be so afraid of bitfields.
- cle 5y agoCan you elaborate on your shallow dismissal?
- dtech 5y agoThis might have been valid in 00's Java/C#. However, most modern languages and even Java 16+ have good Enum support and exhaustiveness checks, making them very convenient and safe to express a limited set of possible values. In contrast, an alternative like polymorphism + a visitor is safe, but excruciatingly verbose and hard to follow and modify.
- flaviu1 5y agoI'd expect it to go something like this: - There are many many languages without classes that make good use of enums - Doing this refactoring is excessively verbose - Someone making bold claims like this about a language feature that's never been considered error prone and been around since the beginning ought to provide some really good evidence to back things up. There's cases where enums are good, and there's cases where polymorphic classes are good. Dismissing one of them by default is wrong, since both are useful at different times. edit: and the citation there to Martin Fowler et al., Refactoring: Improving the Design of Existing Code doesn't even match the claim being made. That book mentions - "The first part of this problem is that switch statement. It is a bad idea to do a switch based on an attribute of another object. If you must use a switch statement, it should be on your own data, not on someone else's." -- sure, I agree - "Often you find the same switch statement scattered about a program in different places. If you add a new clause to the switch, you have to find all these switch, statements and change them. The object-oriented notion of polymorphism gives you an elegant way to deal with this problem." -- fair enough This is a long way from "enums are a code smell," and honestly feels like padding the citation count. Another thing to note is that the second edition of Refactoring: Improving the Design of Existing Code says: > Even in our more wild-eyed youth, we were never unconditionally opposed to the conditional. Indeed, the first edition of this book had a smell entitled “switch statements.” The smell was there because in the late 90’s we found polymorphism sadly underappreciated, and saw benefit in getting people to switch over. > These days there is more polymorphism about, and it isn’t the simple red flag that it often was fifteen years ago.
- masklinn 5y ago> - "Often you find the same switch statement scattered about a program in different places. If you add a new clause to the switch, you have to find all these switch, statements and change them. The object-oriented notion of polymorphism gives you an elegant way to deal with this problem." -- fair enough Even that seems like it could easily be way overkill e.g. if you have multiple switches which, say, generate a label from an enum, the first step is probably to add a utility function / method, not to migrate the whole thing over to polymorphism. Although there is one thing to be said about context: * OP works in C#, whose enums are literally useless (they’re like C’s) * apparently even in Java (which at least has type-safe enums even if not sum types), `switch` is unable to check for completeness
- couchand 5y agoGenerally the issue with the switches scattered about (or more generally, and concern about conditions scattered about the codebase) is that the body of each case is different. Of course if all the bodies are the same the easier option is a utility method.
- jaywalk 5y agoAs someone who works in C# and finds enums useful, I'd be curious to know why you describe them as literally useless.
- masklinn 5y agoA C# enum is, like a C enum (as they were explicitly introduced to be compatible with those), just a bunch of constants for integers. So when you have an enum-typed value, odds are good that it’s one of the named ones but there’s no mechanism anywhere preventing it to be any other integer of the underlying type.
- Semaphor 5y agoBut how is that an issue? I guess when you are casting random integer to the enum-type without any checks?
- strulovich 5y agoFor me it’s the following: - going for polymorphic classes instead of a few switches can cause bad coupling of knowledge. ( For example, instead of a switch on what to log in module A and the same for module B, now you’re mixing code from these two modules due to a shared constant. Sometimes these polymorphic classes make sense, but my experience is that it’s a minority, and it’s better to err on the side of some simple enums. Also note that Uncle Bob in Clean Code pretty much directly opposes in one of the chapters about logic and data separation (can’t find the chapter right now since I don’t have the book anymore)
- coldacid 5y agoMark Seemann started it first.
- kaetemi 5y agoThe OO story generally falls apart once you go outside of the module/library, and want to actually treat objects like generic objects, and have new custom behaviour for some types of objects. Either you end up with 'scary' enums, or with dynamic_cast and 'scary' null pointers.
- mannykannot 5y agoFivelessminutes's dismissal is indeed shallow, but then again, the rule being dismissed looks like shallow dogma (though we are not seeing it in context.) Enums are essentially categorical variables, and there is nothing wrong with that: they are widely used in statistics and other real-world applications (arguably, even, booleans are a case of such.) As a junior programmer, I was once assigned to work on an example of what could happen as a result of following this advice: an explosion of subclasses for the sake of avoiding a few conditional clauses. Algorithms had been broken up so that the common parts could be put in base classes, and you had to jump all around the code to see what was going on. Worse, objects could change their categorization during their lifetime, which required the substitution of a new object for the previous one. This complexity was contagious; once this one type was implemented in this manner, a lot of other things had to follow suit. This was one of the experiences that turned me into a skeptic with regard to the proposition that there's no problem that cannot be simply solved by being more object-oriented.
- bena 5y agoI'd never heard of the phrase "frog in a well" before. I like it, I don't think I ever had a term for that phenomenon before.
- marcosdumay 5y agoIt's a reference to a story about a group of frogs that adapt to living a hellish life in a well, and when given the means to escape decide to destroy it instead of escaping. The story itself seems to be ancient, but I couldn't find where it originates from.
- Jensson 5y agoI think it is originally from China, I see it a lot in Chinese stories and never seen it anywhere else (except here now).
- marcosdumay 5y agoHum... I know it from Portuguese literature. I was able to find some references from ancient India, but nothing about it coming from there.
- smegsicle 5y ago"Kupa manduka is a Sanskrit phrase for the frog in the well that imagines the well, its home, to be the whole world and, therefore, becomes pompous. It is a term of derision for the intellectually complacent, for someone who thinks he knows everything there is to know."
- userbinator 5y agoSeeing sentences like that are very illuminating to explain why a lot of software turns into bloated monstrosities.
- AndrewBissell 5y agoSomehow I knew it must have been a dependency injection advocate who came up with the idea that enums are a code smell.
- quickthrower2 5y agoThe more insidious problem is the card carrying believer in X saying “!X is harmful” See this pattern all the time in both technical and people side all the time. “You should never estimate using time” “Don’t start new work until all the teams sprint commitments are completed” Etc. In a word it is arrogance with a twist of ignorance. Ignorance that complex systems of humans and code don’t follow perfect dogmatic rules. It might be ok for junior devs to be a bit like this but always add a pinch of “most of the time, but when experienced you’ll know when to ignore the rule” Ignoring some unwritten or written dogma is often what makes for a competitive advantage. Let your competitor drown in EnumClassFactorys .
- AndrewBissell 5y agoEvery one of these language toolbox books should come with a disclaimer for new devs up front about how the mark of a good craftsmen is knowing when to use the tools, when to bend or break their rules and guidelines, etc. I know it certainly might have saved me a fair bit of grief and oddly written code.
- userbinator 5y agoBut then they wouldn't sound "authoritative" enough. The dogma-cult that keeps its leader well-fed wouldn't survive. I think the best authoritative-sounding counter-quip is "Best practices are best not practiced."
- quickthrower2 5y ago“Should” is way too strong. “You should consider” is appropriate. An enum is essentially an encoding over integers. I guess this depends on your programming language, so humour me. If you have an integer whose meaning is a category (as opposed to a count or measure) - does this need refactoring into classes? Probably not most of the time. For example you import a CSV and one of the fields is a categorisation. Let’s say county code. Your code may occasionally care if you are in an EU country. You could have a class per country and override isEU as needed but this is clumsy and ugly in my opinion. You’d also need a switch statement, in a factory method to create these from the csv data. A lot of the time it’s easier to follow code where the switch statement is in its intuitive place. Highly OO abstracted code can be very hard to reason about too.
- dahfizz 5y ago> Highly OO abstracted code can be very hard to reason about too. Definitely. Coming up with all the right design patterns and abstractions can feel quite clever and satisfying. But when someone else needs to debug that code, having to jump through layers and layers of object, interfaces, factories, etc to find out what a method call is actually doing is a pain.
- raverbashing 5y agoI find it funny how Java (and maybe .NET, though the example is .NET - but probably "extended" from Java) seems to have catalyzed the worse types of cargo-culting, OO "overdose" and similar pseudo-good practices. Then we end up with "corporate java code" that wants to throw out what was the basis of computing (if statements, switches) and turn all into some crazy type hierarchy.
- Supermancho 5y agoNot so much .NET/Java but Structured Programming (1972 Edsger W. Dijkstra, Ole-Johan Dahl, and Tony Hoare) has prompted the gradual backlash against switches. (https://youtu.be/eEBOvqMfPoI?t=1512 https://youtu.be/eEBOvqMfPoI?t=1512, re: Duff's Device). Senior developers should watch the whole thing to understand how block scope and functions and classes relate. Modern development has drifted toward lots of small functions (which causes a mess because modern IDEs dont have supporting features like this: https://youtu.be/baxtyeFVn3w?t=1786 https://youtu.be/baxtyeFVn3w?t=1786, re: gtoolkit). A switch requires you to look inside and reason about the internals of a function to understand how it will behave before exiting, as the compiler. This doesn't matter so much if you dont care about early exits (no branch prediction, etc). Either way, it's extra cognitive overhead and error prone for a small improvement in readbaility. It's the line where programming becomes tricky programming. As a thought experiment, imagine there is ANOTHER keyword that acts like an if/else (or switch with mandatory breaks). If A. it's terse, B. it's structured, C. it's readable, would that be better? We primarily use switch because it's easy to read (versus a ton of if{}/else{}), as it gives us almost-block scoping without having to define another indirection/function. Python has this switch equivalent in match/case, but you have to use Python to get it. Switch use is a syntax problem. Lots of software is colored by bad syntax and language design^. ^ I think most people realize that type hierarchies are a brittle/limited form of mixin. Even Java has softened to this a bit (aspects, etc) but will likely never be fun to use.
- adamkl 5y agoIn Mark’s defence, our views on software development can change over time as we learn and grow. Mark, for example, is now primarily a functional programmer and makes good use of F#’s type system: https://blog.ploeh.dk/2015/08/10/type-driven-development/ https://blog.ploeh.dk/2015/08/10/type-driven-development/ So he might even agree with you these days (on the “baloney” part, but maybe not the “frog in a well”).
- aardvark179 5y agoI seem to remember the Martin Fowler advice that this apparently cited being slightly more nuanced, and it’s not totally wrong. If you have a simple enumerator and many switch statements depending on it then it is quite error prone if you have to add a value (depending on your language it may even be a silent failure), but a class will force you to implement the bits of behaviour you need as methods, and gives a good template for when you add another subclass. It’s not always the right thing to do, you’re trading the flexibility for others to add new types for the old flexibility of it being easy to add new behavioural decision based on a fixed set of types. I think in a lot of cases it’s definitely worth stopping and thinking, and making sure you are implementing the right thing if you find yourself using enums heavily, or using a class hierarchy as an enumerator replacement.
- dwaltrip 5y agoIf I’m understanding correctly, languages like rust avoid that problem by throwing a compile error when a match statement doesn’t cover all possible cases. Of course, a catch-all would then somewhat reintroduce the problem.
- aardvark179 5y agoYes. The problem you then have is that adding additional values to the enum then either breaks compatibility or relaxes that guarantee. Since Java tries to run old code for as long as possible the only option for switch statements is to ensure there is a fallback so that you at least know there is a problem.
- dang 5y agoOk, but please don't post unsubstantive comments here. In particular, shallow dismissals and calling names are against the site guidelines. "A good critical comment teaches us something." https://news.ycombinator.com/newsguidelines.html https://news.ycombinator.com/newsguidelines.html