If Statements (Part 2)
Two years later....
...I met Aaron, an engineer on the server team, and a modern-day prophet denouncing the evil coding practices of our time!
Aaron had a problem with switch-statements. He said:
Every time you write a switch statement, a little part of Aaron dies.
I asked him to explain this to me. He told me most of the time, when you write a switch statement, you're really switching on the type of something. So, remove the switch statement, and make an actual type in the language, then use polymorphism to accomplish the same job.
Take this pseudocode using OpenGLES for example. OpenGLES has different versions. It's possible to draw a cube in each one, but how you draw a cube varies quite a bit. Maybe you wrote this code in your game:
enum OpenGLVersion {
GLES10 = 1,
GLES20 = 2,
GLES30 = 3
};
class Game {
private:
OpenGLVersion openGLVersion;
void draw() const;
// ...etc etc...
};
void Game::draw() const {
// Code that's common to
// drawing a cube
// using any GL version.
switch(openGlVersion) {
case GLES10:
// draw the cube the GLES10 way.
break;
case GLES10:
// draw the cube the GLES20 way.
break;
case GLES30:
// draw the cube the GLES30 way.
break;
}
}
(A little part of Aaron dies when you do that.)
We're switching on something. Let's take Aaron's advice, and convince ourselves that something is a type. GLES10, GLES20, GLES30 those are different versions of OpenGL... which is a graphics engine. Eureka!
class GraphicsEngine {
virtual void drawCube() const;
protected:
void beginDrawCube() const;
};
class GLES10 : public GraphicsEngine {
void drawCube();
};
class GLES20 : public GraphicsEngine {
void drawCube();
};
class GLES30 : public GraphicsEngine {
void drawCube();
};
void GraphicsEngine::beginDrawCube() const {
// Code that's common to
// drawing a cube
// using any GL version.
}
void GLES10::drawCube() const {
beginDrawCube();
// draw the cube the GLES10 way
}
void GLES20::drawCube() const {
beginDrawCube();
// draw the cube the GLES20 way
}
void GLES30::drawCube() const {
beginDrawCube();
// draw the cube the GLES30 way
}
And finally
class Game {
private:
GraphicsEngine* graphicsEngine;
void draw() const;
// ...etc etc...
};
void Game::draw()
{
graphicsEngine->drawCube(); // The switch is gone!
}
Why is this better?
-
The functions are shorter. There are more of them, but they're shorter. That generally makes the code more readable, since each function is a more easily-digested unit with a descriptive name.
-
It's more debuggable. Suppose you're tracing the origin of a bug that only shows up in OpenGLES 3.0 Imagine stepping through the new code versus code with a switch. The arrow is pointed at
graphicsEngine->drawCube();
, youstep in
, now the arrow is in a new function that only does the OpenGLES 3.0 method. If the other draw functions are in different files, you might not even see them, which is good because you're not debugging them right now. With the switch statement, the other switch cases are cluttering up the screen with code that isn't currently relevant. -
I would argue that the new code reflects a more accurate mental model. The game doesn't draw the cube itself, it appeals to the graphics engine to draw the cube. I think of the graphics engine as being an object. Now, it's an object in the code. Before, the graphics engine was an object in my mind, but in the code, I had a number.
-
If the classes GLES10, GLES20, GLES30 are separated into files, you can optimize the build by only including headers that each file needs. Suppose some day, OpenGLES 1.0 is no longer supported on the platform you're on, and you need to take out the OpenGLES 1.0 code. If you do things right, you just delete a file and you're done.
The thing is, now that I see things Aaron's way, I've started to see all the places this pattern gets violated. I understand the painful sensation Aaron experiences whenever someone in the world writes a switch.
There's no rest for the righteous, I guess.