Topics
Refactoring is the act of improving the structure of a program without changing its behavior. Refactoring is done in
tiny little steps, each barely worth doing. In between each step, we run the relevant unit tests to make sure that the
changes we made have not broken anything. The edit/compile/test cycle is usually between 30 seconds and five minutes.
The purpose of refactoring is to improve the design and readability of the code. There are several specific goals:
-
The code should pass all its tests.
-
It should be as expressive as it is possible for you to make it.
-
It should be as simple as it is possible for you to make it.
-
It should have no redundancy.
Refactoring is not something that we schedule. There is no line item in the schedule for it. There is no particular
time when we do it. Refactoring is done all the time. As you and your pair partner work on a task, such as writing
tests and code, you will notice that the code and tests are not as clean and simple as they could be. That is the time
to stop and refactor that code.
The rule is: Don't let the sun set on bad code.
Consider the two unit tests and the Formatter class shown below. The Formatter class works but is not as expressive as I'd like it to be. So I'll refactor it in
stages.
public void testCenterLine()
{
Formatter f = new Formatter();
f.setLineWidth(10);
assertEquals(" word ", f.center("word"));
}
public void testOddCenterLine() throws Exception
{
Formatter f = new Formatter();
f.setLineWidth(10);
assertEquals(" hello ", f.center("hello"));
}
|
import java.util.Arrays;
public class Formatter
{
private int width;
private char spaces[];
public void setLineWidth(int width)
{
this.width = width;
spaces = new char[width];
Arrays.fill(spaces, ' ');
}
public String center(String line)
{
int remainder = 0;
StringBuffer b = new StringBuffer();
int padding = (width - line.length()) / 2;
remainder = line.length() % 2;
b.append(spaces, 0, padding);
b.append(line);
b.append(spaces, 0, padding + remainder);
return b.toString();
}
}
|
The setLineWidth function is a little mysterious. What is this spaces array and why is it being filled with blanks? By looking ahead into the center function, we see that the spaces array is just a
convenience to allow us to move a number of blanks into a StringBuffer. I wonder if we
really need this convenience array.
For the moment, I'm going to pull the initialization of the array out into its own function named buildArrayOfSpaces. That way, it's all in one place, and I can think about it a little more
clearly.
public void setLineWidth(int width)
{
this.width = width;
buildArrayOfSpaces(width);
}
private void
buildArrayOfSpaces(int width)
{
spaces = new char[width];
Arrays.fill(spaces, ' ');
}
Run the tests: the tests pass
|
I don't like the way center function is constructed. There is math scattered all through
it. I think we can rearrange the math to make things clearer.
public String center(String line)
{
int remainder = line.length() % 2;
int numberOfBlanksInFront = (width - line.length()) / 2;
int numberOfBlanksAtEnd = (width - line.length()) / 2 + remainder;
StringBuffer b = new StringBuffer();
b.append(spaces, 0,
numberOfBlanksInFront);
b.append(line);
b.append(spaces, 0,
numberOfBlanksAtEnd);
return b.toString();
}
Run the tests: the tests pass
|
This looks good, but we can reduce the clutter by changing some of the variables into functions.
public String center(String line)
{
StringBuffer b = new StringBuffer();
b.append(spaces, 0,
numberOfBlanksInFront(line));
b.append(line);
b.append(spaces, 0,
numberOfBlanksBehind(line));
return b.toString();
}
private int numberOfBlanksBehind(String line)
{
int extraBlankIfOdd = line.length() % 2;
return (width - line.length()) / 2 + extraBlankIfOdd;
}
private int numberOfBlanksInFront(String line)
{
return (width - line.length()) / 2;
}
Run the tests: the tests pass
|
This makes the center function read a little better. However, the use of the StringBuffer.append function is a little confusing. We might be able to improve it a little by
creating a more explicit function.
public String center(String line)
{
StringBuffer b = new StringBuffer();
appendBlanks(b, numberOfBlanksInFront(line));
b.append(line);
appendBlanks(b, numberOfBlanksBehind(line));
return b.toString();
}
private void appendBlanks(StringBuffer b, int numberOfBlanks)
{
b.append(spaces, 0, numberOfBlanks);
}
Run the tests: the tests pass
|
Now we can rewrite appendBlanks to avoid using the spaces
array.
import java.util.Arrays;
public class Formatter
{
private int width;
public void setLineWidth(int width)
{
this.width = width;
}
public String center(String line)
{
StringBuffer b = new StringBuffer();
appendBlanks(b, numberOfBlanksInFront(line));
b.append(line);
appendBlanks(b, numberOfBlanksBehind(line));
return b.toString();
}
private void appendBlanks(StringBuffer b, int numberOfBlanks)
{
while(numberOfBlanks-- > 0)
b.append(' ');
}
private int numberOfBlanksBehind(String line)
{
int extraBlankIfOdd = line.length() % 2;
return (width - line.length()) / 2 + extraBlankIfOdd;
}
private int numberOfBlanksInFront(String line)
{
return (width - line.length()) / 2; }
}
}
Run the tests: the tests pass
|
The names of functions like numberOfBlanksBehind imply that the reader knows that these
will be called from the center function. We should eliminate this implication by
renaming those functions.
import java.util.Arrays;
public class Formatter
{
private int width;
public void setLineWidth(int width)
{
this.width = width;
}
public String center(String line)
{
StringBuffer b = new StringBuffer();
appendBlanks(b,
numberOfBlanksToLeftOfCenter(line));
b.append(line);
appendBlanks(b,
numberOfBlanksToRightOfCenter(line));
return b.toString();
}
private void appendBlanks(StringBuffer b, int numberOfBlanks)
{
while(numberOfBlanks-- > 0)
b.append(' ');
}
private int
numberOfBlanksToRightOfCenter(String line)
{
int extraBlankIfOdd = line.length() % 2;
return (width - line.length()) / 2 + extraBlankIfOdd;
}
private int
numberOfBlanksToLeftOfCenter(String line)
{
return (width - line.length()) / 2;
}
}
Run the tests: the tests pass
|
And I think we are done. You might find other refactorings to do, or you might not agree with all of the refactorings
I've done. That's to be expected. The point is, however, that I have put a lot of effort into the readability and
simplicity of this class. That effort will help others understand this class and make it easier for them to change the
class when the time comes.
|
