Refactor: Extract If statements to Enum

When a method's logic gets complex it can indicate we are missing an abstraction.  In this entry I highlight how Java's enum can be a simple abstraction that simplifies your logic and improves
code readability.  As a starting point I use a class from the bowling game kata.

Additional background information about the bowling game kata can be found here The Bowling Game Kata.

The refactoring step applied introduces an enum to represent the frame type and moves the scoring logic to the enum instance.  This simple technique can be used to simplify code by adding simple abstractions.

I'm going to focus my refactoring on the score method (which scores a bowling games based on the frames bowled) that at the end of the kata looks like this:

	public int score() {
	int score = 0;
	int frameIndex = 0;
	for (int frame = 0; frame < 10; frame++) {
	if (isStrike(frameIndex)) {
	score += 10 + rolls[frameIndex+1] + rolls[frameIndex+2];
	frameIndex++;
	} else if (isSpare(frameIndex)) {
	score += 10 + rolls[frameIndex+2];
	frameIndex += 2;
	} else {
	score += rolls[frameIndex] + rolls[frameIndex+1];
	frameIndex += 2;
	}
	}
	return score;
	}

view raw origScore.java hosted with ❤ by GitHub

The score method has a nested if else statement that I would like to simplify. Often, when code seems complex it can indicate you are missing a concept. In this case I think introducing a class that represents the type of frame reduces the complexity and makes the code easier to understand. The FrameType enum represents how the player scored in a Frame - Strike, Spare or Open (meaning not all the pins were knocked down). The classifyFrame method determines the applicable type based on one or two rolls.

	public enum FrameType {
	STRIKE,
	SPARE,
	OPEN;
	public static FrameType classifyFrame(int firstRoll, int secondRoll) {
	if (firstRoll == MAX_PINS) {
	return STRIKE;
	}
	if (firstRoll + secondRoll == MAX_PINS) {
	return SPARE;
	}
	return OPEN;
	}
	}

view raw FrameType1.java hosted with ❤ by GitHub

Now that I can classify the frame I think the enum should know how to compute the score for the frame. Since each FrameType instance has a slightly different score calculation I am defining an abstract method within the enum. Then each enum instance will define its specific implementation.

	public enum FrameType {
	STRIKE {
	public int scoreFrame(int rolls[], int frameIndex) {
	return MAX_PINS + rolls[frameIndex+1] + rolls[frameIndex+2];
	}
	},
	SPARE {...},
	OPEN {...};
	public static FrameType classifyFrame(int firstRoll, int secondRoll) {...}
	public abstract int scoreFrame(int[] rolls, int frameIndex);
	}

view raw FrameType2.java hosted with ❤ by GitHub

This is the final version of the class. The rollsInFrame method is another behavior that is specific to each frame type. As you can see this refactoring removed the if else structure from the method. The score method now highlights the top level logic and the details are in the enum.

	public class BowlingGame {
	public static final int MAX_PINS = 10;
	public enum FrameType {
	STRIKE {
	public int scoreFrame(int rolls[], int frameIndex) {
	return MAX_PINS + rolls[frameIndex+1] + rolls[frameIndex+2];
	}
	public int rollsInFrame() {
	return 1;
	}
	},
	SPARE {
	public int scoreFrame(int rolls[], int frameIndex) {
	return MAX_PINS + rolls[frameIndex+2];
	}
	public int rollsInFrame() {
	return 2;
	}
	},
	OPEN {
	public int scoreFrame(int rolls[], int frameIndex) {
	return rolls[frameIndex] + rolls[frameIndex+1];
	}
	public int rollsInFrame() {
	return 2;
	}
	};
	public static FrameType classifyFrame(int firstRoll, int secondRoll) {
	if (firstRoll == MAX_PINS) {
	return STRIKE;
	}
	if (firstRoll + secondRoll == MAX_PINS) {
	return SPARE;
	}
	return OPEN;
	}
	public abstract int scoreFrame(int[] rolls, int frameIndex);
	public abstract int rollsInFrame();
	}
	int[] rolls = new int[21];
	int rollIndex = 0;
	public void roll(int pinsKnockedDown) {
	rolls[rollIndex] = pinsKnockedDown;
	rollIndex++;
	}
	public int score() {
	int score = 0;
	int frameIndex = 0;
	for (int frame = 0; frame < 10; frame++) {
	FrameType frameType = classifyFrame(rolls[frameIndex], rolls[frameIndex + 1]);
	score += frameType.scoreFrame(rolls, frameIndex);
	frameIndex += frameType.rollsInFrame();
	}
	return score;
	}
	}

view raw BowlingGame1.java hosted with ❤ by GitHub

Introducing an enum is a simple way to encapsulate different logic into objects versus using if statements (a OO technique called polymorphism). In the right context this is a powerful tool to simplify and improve code readability.  Using an enum is a simple way to introduce a new object and harness polymorphism.  If your needs evolve the implementation can become classes and that inherit from a FrameType super class.  There is nothing in the code that calls the scoreFrame method specific to our chosen enum solution.

At a high-level most people know that in bowling spares and strikes are scored differently. The BowlingGame class now tells that story.

Can we do better?

The score method seems ok - it is expressive, the level of abstraction is the same for each statement.  I don't always like using the += operator.  Why is the enum called FrameType? Is there a better name?

The FrameType enum makes sense, I think it reads fine.  It does seem horizontally challenged.  There are a lot of statements, somewhat due to Java language constructs.  What other options do we have?  Maybe functional lambdas?