Hacker News Reader

Comment by pcwalton | Hacker News Reader

> Professionally, I have run into programs that have massive switch statements with hundreds of cases in them, which I've always considered a "code smell".

Most optimizing compilers will emit a jump table for a switch, which is a good way to get a reasonably high performance interpreter while keeping the code pretty clean. The performance is better than using virtual methods [1].

[1]: http://www.complang.tuwien.ac.at/forth/threading/

View on HN

It's still a code smell. It might be a justifiable code smell in some very specific circumstances where the performance actually matters, but a code smell nevertheless.

You are substituting dogma for critical thinking. Sometimes a switch statement (or goto, etc.) is the best overall solution to a problem. In those cases, it is actually good code, not "justifiable code smell."

"Code smell" is just a heuristic. It's what happens when you look at some code and something in your head says "this doesn't look good". We all know that in the real world maintainability is more important than minor theoretical performance gains (yes, premature optimization without actually ever measuring the gains is a common thing, as we all hopefully know). I certainly didn't mean to imply that a switch or a goto statement cannot be the best way to implement something; I simply meant that a theoretical performance argument is not enough to make good code out of bad code.

A large switch statement is exactly the correct solution for this task. Other virtual machines do the same thing.

For example, look at Python's ceval.c, which has a switch table implementation and the text "The traditional bytecode evaluation loop uses a "switch" statement, which decent compilers will optimize as a single indirect branch instruction combined with a lookup table of jump addresses."

Actually, it also has a version which uses GCC's "Labels as Values" extension, see http://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html . The comment then points out that this non-portable solution is up to 15-20% faster than a large switch statement, because having N jump points instead of 1 improves the CPU's branch prediction.

As haberman says, this is "actually good code", not "justifiable" bad code.

I am another who dislikes the term "code smell." I agree with mbrock - I would rather get a comment which reflects the underlying complaint than use a proxy term like "code smell". In your reply to mbrock you wrote:

> the "smell" is just a heuristic that says that without any additional arguments in its favor, a hundred-case switch usually isn't the cleanest and most maintainable way to implement something.

Why not just say "a hundred-case switch statement usually isn't ...", and omit a reference to "smell"? What additional meaning does the term "code smell" lend?

I dislike using the term "code smell" in general. Some people detest stinky cheeses, or a peaty whisky, while others adore the complex aromas. Often this is a learned taste which comes with age and experience.

As a result, the obvious rebuke to any "this is a code smell" comment is "that's because you aren't mature enough to appreciate it." I can't think of any effective response which stays on topic, other than to bring the conversation back to the specific problems in the code. Why not just start from that point instead?

I didn't even say anything about performance here. Even ignoring performance, sometimes switch() is best.

For example, I wrote a very small JIT compiler for Brainfuck. It uses switch() to do code generation. It's extremely clean and elegant code, if I do say so myself. Breaking apart the switch() with some kind of OO approach would make the program longer and less clear: http://blog.reverberate.org/2012/12/hello-jit-world-joy-of-s...

Do you look at that and think "this doesn't look good" just because it uses switch()? If so, your heuristic could use a little tuning.

But now let's talk performance. In interpreter/VM main loops, bytecode dispatch is a huge factor in the overall performance of a VM. This issue isn't "theoretical," "minor," or "premature," it's the result of extensive measurement and experimentation by many people over a large amount of time. If you are not familiar with this well-studied issue, here is some recommended reading:

http://software.intel.com/en-us/forums/topic/298506

http://lua-users.org/lists/lua-l/2011-02/msg00742.html

http://www.cs.tcd.ie/publications/tech-reports/reports.07/TC... (see section 3.3)

Note that x6502 also uses goto :)

You have often a (single) big switch in lexers (often automatically generated). In network communication, you have often a big switch on the type of message (less often automatically generated).

It is not a bad pattern to have switch in those cases, but it is often present in code analyse rules. Code analyse tools are very bad to distinguish between good use of a feature that is rarely used and missuse of this feature.

A smell is an indicator that something might be wrong, not that it is a sub-optimal way of doing things.

If you create code that smells, you should justify it (in comments or other documentation). Likewise just saying it smells without suggesting an alternative is unhelpful to the discussion.

Why not point out concrete problems with the implementation? We can't stop at the "smells" level of discussion! So, for example, is it hard to understand? Does it perform poorly? Is it difficult to extend?

It depends on the context. The context may make the smell go away; the "smell" is just a heuristic that says that without any additional arguments in its favor, a hundred-case switch usually isn't the cleanest and most maintainable way to implement something.

Can you define the criteria by which you judge it to be a 'code smell'?

Can you contextualise the criteria? Can you match those criteria to this context?

(Not rhetorical, I'd like to know the answer.)

Reply on news.ycombinator.com