4 ms·
The association between the two has to be written somewhere. Can you provide an alternative approach that isn't vulnerable to typos?
by Twisol 6y ago
The association between the two has to be written somewhere. Can you provide an alternative approach that isn't vulnerable to typos?
- Jtsummers 6y agoTheir point seems to be in the mismatch of names. Imagine seeing this sequence, with names instead of numbers: switch(HTTP_METHOD) { case PUT: processPut(...); case POST: processPost(...); case GET: processDelete(...); // wtf case DELETE: processGet(...); // mate? } To the reader that appears to be an error even if it is precisely the thing you want to happen.
- Leherenn 6y agoDepends on your language, but you could use the type system instead; e.g. the function is chosen by the compiler based on the type of the value. In more dynamic languages, you could probably use introspection. Lastly, this does not alleviate the association issue, but I prefer the alternative of declaring the associations in an array/map somewhere, and using map[enum_value]() instead of the switch.
- slaymaker1907 6y agoIn a lot of ways, I think the map solution is actually worse. There isn't an easy way to figure out what code is used for a given enum value with the map alone. The searchability depends on how the map is initialized. Not to mention a map will always introduce null or option types that you have to handle. A map can be a good solution though, particularly if it's something like mapping enum values to strings that is constructed via EnumClass.values().map... or something so that you know the map is a total function.
- gurkendoktor 6y agoUsing a map also completely sidesteps the point of cyclomatic complexity because there are no code paths to count anymore; now they're data. And even though the function does the same as before, the linter will congratulate you on the reduced complexity.
- Twisol 6y agoAnd you'd have essentially replaced statically understood control flow with the equivalent of a computed goto. It's not like it's a bad approach, but from a code path standpoint, it's unequivocally worse, not better. (Imagine if that map was mutable!)
- Leherenn 6y ago> Not to mention a map will always introduce null or option types that you have to handle. It is the same with the switch, at least in C/C++. You either have a default, or must list all the possible cases and still return something "at the bottom".
- slaymaker1907 6y agoThis is becoming an increasingly less common strategy for languages to take given the popularity of sum types.
- wvenable 6y agoThis is basic DRY violation. The number is duplicated twice and that can get out of sync (as in the example). I think everyone has coded this way out of expedience and some of us have eventually messed it up too. But you could, for example, use a macro in C to only list the number once. It might not be a trade-off worth making though. Personally, I've used reflection to do this kind of mapping of enums to functions in a language that supports reflection.
- Twisol 6y ago> This is basic DRY violation. The number is duplicated twice 2 and that can get out of sync (as in the example). I think it's pretty clear that the number is a placeholder for something reasonable, e.g. making an association between two distinct sets of concepts. You'll still be vulnerable to copy-paste or typo issues. > Personally, I've used reflection Now you have two problems (and still have to maintain an association between two sets of concepts).
- wvenable 6y agoI don't think most enum-to-function mappings are distinct sets of concepts. In my own code, I have used the enum name to map to a set of functions related to that enum value. The association is implicit in the name of the enum value and the name of the functions. No way to mess that up like this. If it was: case: KIND_BOX: return parseObjectOfKindBox(); It's no difference. Still repeating "Box".
- gurkendoktor 6y agoThe code was inspired by a piece of Java code that rendered geometric primitives. It was basically case "box": return parseBox(); as you suggested in your other post. Turning "box" into "parseBox" using string operations and then using reflection to call the right method is an approach I'd consider in Ruby, but not in Java. It breaks IDE features like "Find Usages" and static analysis, and the code to dynamically invoke a Java method is more annoying to review than the boring repetition in my posted snippet.