ArticleS. UncleBob.
ExtractClass [add child]

Extract Class

I was writing Hunt The Wumpus again as an exercise for a two day FitNesse course I'm teaching. As you may know, HTW is an old adventure game that was made popular in the 70s in the book 101 Basic Games. I was writing it as a console application.

At first I just had this class named Game that dealt with the console and the game rules. But as the program grew larger, I realized that I needed to separate the game rules from the console management. There was no need for the game rules to know about invalid commands, or about the messages that were printed on the console, or about the syntax of the commands. So I decided to refactor the Game class into two classes named GameRules and GamePresenter. The GameRules class would know nothing of the syntax of the commands and messages. It would only know about rules. The GamePresenter would know nothing of game rules, it would only know about how to parse commands, and print messages.

It seemed to me that this ought to be an automatic refactoring that IntelliJ (my IDE of choice) ought to provide. But it doesn't. I balefully scanned the refactoring menu for something like extract class but there was nothing there. But then I saw Extract Superclass in the refactoring menu, and I realized that if I could separate the GameRules into a base class, and derive the GamePresenter from it; then breaking the two into separate classes would be a lot simpler.

Then I realized that there was another refactoring that I had often wondered about named Replace inheritance with delegation.. Could it be? Might the two, together, equal Extract class? Yes! I was able to combine the two so neatly, that I decided to write this up as a blog.

To demonstrate the concept, I've created a little console application based on rolling dice for Dungeons and Dragons. In the DnD literature they use a simple scheme for telling you what kind of dice to roll. For example: 2d4 means roll the four sided die twice and sum the rolls. DnD literature is full of these little abbreviations. D6 means roll a six sided die. 2d20 means roll the 20 sided die twice and sum the rolls. So I wrote a cute little application that allows you to type a DnD dice command, and that prints the result. Here it is:


import java.io.*;

public class DNDDice {
public int roll(int n, int d) {
int sum = 0;
for (int i = 0; i < n; i++) {
sum += roll(d);
}
return sum;
}

private int roll(int d) {
return (int) (Math.random() * d + 1);
}

public int[] parse(String commandString) {
int[] command = new int[]{1, 6};
commandString = commandString.toLowerCase();
String[] elements = commandString.split("d");
if (elements[0].length() > 0)
command[0] = Integer.parseInt(elements[0]);
if (elements[1].length() > 0)
command[1] = Integer.parseInt(elements[1]);
return command;
}

public static void main(String[] args) throws IOException {
DNDDice dice = new DNDDice();
BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
while (true) {
String command = br.readLine();
dice.executeCommand(command);
}
}

private void executeCommand(String command) {
int[] diceCommand = parse(command);
int result = roll(diceCommand[0], diceCommand[1]);
displayResult(command, result);
}

private void displayResult(String command, int result) {
System.out.println(command + "=" + result);
}
}

As you can see, the dice rules are comingled with the parsing of the commands, and the printing of results. I'd like to separate these. So first I use the Extract Superclass refactoring to split the class into two classes related by inheritance. This leaves me with two classes as follows:

public class DiceRules {
public int roll(int n, int d) {
int sum = 0;
for (int i = 0; i < n; i++) {
sum += roll(d);
}
return sum;
}

private int roll(int d) {
return (int) (Math.random() * d + 1);
}
}


import java.io.*;

public class DNDDice extends DiceRules {

public int[] parse(String commandString) {
int[] command = new int[]{1, 6};
commandString = commandString.toLowerCase();
String[] elements = commandString.split("d");
if (elements[0].length() > 0)
command[0] = Integer.parseInt(elements[0]);
if (elements[1].length() > 0)
command[1] = Integer.parseInt(elements[1]);
return command;
}

public static void main(String[] args) throws IOException {
DNDDice dice = new DNDDice();
BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
while (true) {
String command = br.readLine();
dice.executeCommand(command);
}
}

private void executeCommand(String command) {
int[] diceCommand = parse(command);
int result = roll(diceCommand[0], diceCommand[1]);
displayResult(command, result);
}

private void displayResult(String command, int result) {
System.out.println(command + "=" + result);
}
}

Now I should be able to complete the separate by using the Replace inheritance with delegation refactoring. But - no - it won't let me do it. IntelliJ complains that my unit tests are using the DiceRules.roll method through a DNDDice variable. So I suppose I should refactor my tests.

At this point I should show you my unit tests. BEWARE, these unit tests are not for the faint of heart!

import junit.framework.TestCase;

public class DNDDiceTest extends TestCase {
private static final int MEAN = 0;
private static final int STDDEV = 1;
private DNDDice dice;

protected void setUp() throws Exception {
dice = new DNDDice();
}

public void testRollOne() throws Exception {
double stats[] = roll(1,6,10000);
assertBetween(3.4, stats[MEAN], 3.6);
assertBetween(1.69, stats[STDDEV], 1.73);
}

public void testRolltwo() throws Exception {
double stats[] = roll(2,6,1000);
assertBetween(6.5, stats[MEAN], 7.5);
assertBetween(2.2, stats[STDDEV], 2.55);
}

private void assertBetween(double low, double v, double hi) {
assertTrue("" + low + "<" + v + "<" + hi, v>low && v<hi);
}


private double[] roll(int n, int d, int repeat) {
int[] results = collectResults(repeat, n, d);
return meanAndSigma(results);
}

private int[] collectResults(int repeat, int n, int d) {
int[] results = new int[repeat];
for (int i = 0; i < results.length; i++)
results[i] = dice.roll(n,d);
return results;
}

private double[] meanAndSigma(int[] results) {
return new double[] {mean(results), sigma(results, mean(results))};
}

private double mean(int[] results) {
int sum = 0;
for (int i : results)
sum += i;
return (double)sum / (double) results.length;
}

private double sigma(int[] results, double mean) {
double variance = 0;
for (int i : results) {
double diff = mean - i;
double diff2 = diff * diff;
variance += diff2 / (results.length-1);
}
return Math.sqrt(variance);
}

public void testParseTwoComponentCommand() throws Exception {
int[] rollCommand = dice.parse("2d6");
assertEquals(2,rollCommand.length);
assertEquals(2, rollCommand[0]);
assertEquals(6, rollCommand[1]);
}

public void testParseOneComponentCommand() throws Exception {
int[] rollCommand = dice.parse("d4");
assertEquals(2,rollCommand.length);
assertEquals(1, rollCommand[0]);
assertEquals(4, rollCommand[1]);
}
}

OK, so I overdid it a little. But I was intrigued about testing something as random as dice. I figured that mean and standard deviation were pretty good ways to show that I was indeed summing the random dice correctly... And this is all just for fun, anyway.

Anyway, I need to split the tests up into two different test files. One for the rules, and one for the presentation. This is trivial.

import junit.framework.TestCase;

public class DiceRulesTest extends TestCase {
private static final int MEAN = 0;
private static final int STDDEV = 1;
private DiceRules dice;

protected void setUp() throws Exception {
dice = new DiceRules();
}

...
}


import junit.framework.TestCase;

public class DicePresenterTest extends TestCase {
private DNDDice dice;
protected void setUp() throws Exception {
dice = new DNDDice();
}

public void testParseTwoComponentCommand() throws Exception {
int[] rollCommand = dice.parse("2d6");
assertEquals(2,rollCommand.length);
assertEquals(2, rollCommand[0]);
assertEquals(6, rollCommand[1]);
}

public void testParseOneComponentCommand() throws Exception {
int[] rollCommand = dice.parse("d4");
assertEquals(2,rollCommand.length);
assertEquals(1, rollCommand[0]);
assertEquals(4, rollCommand[1]);
}
}

OK, now I should be able to use the Replace inheritance with delegation refactoring:

import java.io.*;

public class DNDDice {
private final DiceRules diceRules = new DiceRules();

public int[] parse(String commandString) {
int[] command = new int[]{1, 6};
commandString = commandString.toLowerCase();
String[] elements = commandString.split("d");
if (elements[0].length() > 0)
command[0] = Integer.parseInt(elements[0]);
if (elements[1].length() > 0)
command[1] = Integer.parseInt(elements[1]);
return command;
}

public static void main(String[] args) throws IOException {
DNDDice dice = new DNDDice();
BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
while (true) {
String command = br.readLine();
dice.executeCommand(command);
}
}

private void executeCommand(String command) {
int[] diceCommand = parse(command);
int result = diceRules.roll(diceCommand[0], diceCommand[1]);
displayResult(command, result);
}

private void displayResult(String command, int result) {
System.out.println(command + "=" + result);
}
}

Gorgeous! Now I can use the exact same approach to get main into its own class. First I extract the base class named DicePresenter[?]

public class DicePresenter {
private final DiceRules diceRules = new DiceRules();

public int[] parse(String commandString) {
int[] command = new int[]{1, 6};
commandString = commandString.toLowerCase();
String[] elements = commandString.split("d");
if (elements[0].length() > 0)
command[0] = Integer.parseInt(elements[0]);
if (elements[1].length() > 0)
command[1] = Integer.parseInt(elements[1]);
return command;
}

protected void executeCommand(String command) {
int[] diceCommand = parse(command);
int result = diceRules.roll(diceCommand[0], diceCommand[1]);
displayResult(command, result);
}

private void displayResult(String command, int result) {
System.out.println(command + "=" + result);
}
}


import java.io.*;

public class DNDDice extends DicePresenter {

public static void main(String[] args) throws IOException {
DicePresenter dicePresenter = new DNDDice();
BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
while (true) {
String command = br.readLine();
dicePresenter.executeCommand(command);
}
}
}

And then I replace inheritance with delegation again. But - no - it didn't work quite right. Again, the tests mention DNDDice and don't know what to do when the inheritance relationship is broken.

public class DicePresenterTest extends TestCase {
private DicePresenter dicePresenter;
protected void setUp() throws Exception {
dicePresenter = new DNDDice();
}

...
}


Secondly, the DNDDice class has some odd structures left in it.

import java.io.*;

public class DNDDice {
private final DicePresenter dicePresenter = new DicePresenter();

public static void main(String[] args) throws IOException {
DicePresenter dicePresenter = new DNDDice().dicePresenter;
BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
while (true) {
String command = br.readLine();
dicePresenter.executeCommand(command);
}
}
}

But this is all pretty easy to clean up. so...

import junit.framework.TestCase;

public class DicePresenterTest extends TestCase {
private DicePresenter dicePresenter;
protected void setUp() throws Exception {
dicePresenter = new DicePresenter();
}

...
}


import java.io.*;

public class DNDDice {
public static void main(String[] args) throws IOException {
DicePresenter dicePresenter = new DicePresenter();
BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
while (true) {
String command = br.readLine();
dicePresenter.executeCommand(command);
}
}
}

This is much better.
!commentForm

 Tue, 25 Oct 2005 20:32:05, Michael Feathers,
Neat. I really like Extract Class. I think it is one of the safest refactorings to do manually. It's very hard to screw up. You just create a reference to an empty class in your class and move things over in reverse dependency order, making them public as you go. Whenever you move a field or a method, the compiler errors tell you what references you have to change in the original class. After you've moved all you want to you go back and correct the visibilities.