Google Git
Sign in
eclipse / epf / org.eclipse.epf.libraries / fb381d733bfdd08f4332a5a1569689c5bdd43364 / . / XP / xp / guidances / guidelines / refactoring.xmi
blob: f82d991c279ab8b24ee9efdb26f01cb1c463a2e9 [file] [log] [blame]
<?xml version="1.0" encoding="UTF-8"?>
<org.eclipse.epf.uma:ContentDescription xmi:version="2.0" xmlns:xmi="http://www.omg.org/XMI" xmlns:org.eclipse.epf.uma="http://www.eclipse.org/epf/uma/1.0.3/uma.ecore" epf:version="1.0.0" xmi:id="-dbA7zKOJY5WPZyLXErA9vw" name="refactoring,8.137126904637637E-306" guid="-dbA7zKOJY5WPZyLXErA9vw" changeDate="2006-11-09T16:45:22.634-0800" version="1.0.0">
<mainDescription>&lt;h3&gt;
Topics
&lt;/h3&gt;
&lt;ul&gt;
&lt;li&gt;
&lt;a href=&quot;#WhatIs&quot;&gt;What is refactoring?&lt;/a&gt;
&lt;/li&gt;
&lt;li&gt;
&lt;a href=&quot;#Why&quot;&gt;Why should I refactor?&lt;/a&gt;
&lt;/li&gt;
&lt;li&gt;
&lt;a href=&quot;#When&quot;&gt;When should I refactor?&lt;/a&gt;
&lt;/li&gt;
&lt;li&gt;
&lt;a href=&quot;#Example&quot;&gt;An example of refactoring&lt;/a&gt;
&lt;/li&gt;
&lt;/ul&gt;
&lt;h3&gt;
&lt;a id=&quot;WhatIs&quot; name=&quot;WhatIs&quot;&gt;What is refactoring?&lt;/a&gt;
&lt;/h3&gt;
&lt;p&gt;
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.
&lt;/p&gt;
&lt;h3&gt;
&lt;a id=&quot;Why&quot; name=&quot;Why&quot;&gt;Why should I refactor?&lt;/a&gt;
&lt;/h3&gt;
&lt;p&gt;
The purpose of refactoring is to improve the design and readability of the code. There are several specific goals:
&lt;/p&gt;
&lt;ul&gt;
&lt;li&gt;
The code should pass all its tests.
&lt;/li&gt;
&lt;li&gt;
It should be as expressive as it is possible for you to make it.
&lt;/li&gt;
&lt;li&gt;
It should be as simple as it is possible for you to make it.
&lt;/li&gt;
&lt;li&gt;
It should have no redundancy.
&lt;/li&gt;
&lt;/ul&gt;
&lt;h3&gt;
&lt;a id=&quot;When&quot; name=&quot;When&quot;&gt;When should I refactor?&lt;/a&gt;
&lt;/h3&gt;
&lt;p&gt;
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.
&lt;/p&gt;
&lt;p&gt;
The rule is: &lt;b&gt;Don't let the sun set on bad code.&lt;/b&gt;
&lt;/p&gt;
&lt;h3&gt;
&lt;a id=&quot;Example&quot; name=&quot;Example&quot;&gt;An Example of Refactoring&lt;/a&gt;
&lt;/h3&gt;
&lt;p&gt;
Consider the two unit tests and the &lt;font size=&quot;3&quot;&gt;&lt;tt&gt;Formatter&lt;/tt&gt;&lt;/font&gt; class shown below. The &lt;font
size=&quot;3&quot;&gt;&lt;tt&gt;Formatter&lt;/tt&gt;&lt;/font&gt; class works but is not as expressive as I'd like it to be. So I'll refactor it in
stages.
&lt;/p&gt;
&lt;table width=&quot;100%&quot; border=&quot;1&quot;&gt;
&lt;tbody&gt;
&lt;tr&gt;
&lt;td&gt;
&lt;pre&gt;
public void testCenterLine()
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
Formatter f = new Formatter();
&lt;/pre&gt;
&lt;pre&gt;
f.setLineWidth(10);
&lt;/pre&gt;
&lt;pre&gt;
assertEquals(&quot; word &quot;, f.center(&quot;word&quot;));
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
public void testOddCenterLine() throws Exception
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
Formatter f = new Formatter();
&lt;/pre&gt;
&lt;pre&gt;
f.setLineWidth(10);
&lt;/pre&gt;
&lt;pre&gt;
assertEquals(&quot; hello &quot;, f.center(&quot;hello&quot;));
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;/td&gt;
&lt;/tr&gt;
&lt;/tbody&gt;
&lt;/table&gt;
&lt;br /&gt;
&lt;br /&gt;
&lt;table width=&quot;100%&quot; border=&quot;1&quot;&gt;
&lt;tbody&gt;
&lt;tr&gt;
&lt;td&gt;
&lt;pre&gt;
import java.util.Arrays;
&lt;/pre&gt;
&lt;pre&gt;
public class Formatter
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
private int width;
&lt;/pre&gt;
&lt;pre&gt;
private char spaces[];
&lt;/pre&gt;
&lt;pre&gt;
public void setLineWidth(int width)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
this.width = width;
&lt;/pre&gt;
&lt;pre&gt;
spaces = new char[width];
&lt;/pre&gt;
&lt;pre&gt;
Arrays.fill(spaces, ' ');
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
public String center(String line)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
int remainder = 0;
&lt;/pre&gt;
&lt;pre&gt;
StringBuffer b = new StringBuffer();
&lt;/pre&gt;
&lt;pre&gt;
int padding = (width - line.length()) / 2;
&lt;/pre&gt;
&lt;pre&gt;
remainder = line.length() % 2;
&lt;/pre&gt;
&lt;pre&gt;
b.append(spaces, 0, padding);
&lt;/pre&gt;
&lt;pre&gt;
b.append(line);
&lt;/pre&gt;
&lt;pre&gt;
b.append(spaces, 0, padding + remainder);
&lt;/pre&gt;
&lt;pre&gt;
return b.toString();
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;/td&gt;
&lt;/tr&gt;
&lt;/tbody&gt;
&lt;/table&gt;
&lt;p&gt;
The &lt;font size=&quot;3&quot;&gt;&lt;tt&gt;setLineWidth&lt;/tt&gt;&lt;/font&gt; function is a little mysterious. What is this &lt;font
size=&quot;3&quot;&gt;&lt;tt&gt;spaces&lt;/tt&gt;&lt;/font&gt; array and why is it being filled with blanks? By looking ahead into the &lt;font
size=&quot;3&quot;&gt;&lt;tt&gt;center&lt;/tt&gt;&lt;/font&gt; function, we see that the &lt;font size=&quot;3&quot;&gt;&lt;tt&gt;spaces&lt;/tt&gt;&lt;/font&gt; array is just a
convenience to allow us to move a number of blanks into a &lt;font size=&quot;3&quot;&gt;&lt;tt&gt;StringBuffer&lt;/tt&gt;&lt;/font&gt;. I wonder if we
really need this convenience array.
&lt;/p&gt;
&lt;p&gt;
For the moment, I'm going to pull the initialization of the array out into its own function named &lt;font
size=&quot;3&quot;&gt;&lt;tt&gt;buildArrayOfSpaces&lt;/tt&gt;&lt;/font&gt;. That way, it's all in one place, and I can think about it a little more
clearly.
&lt;/p&gt;
&lt;table width=&quot;100%&quot; border=&quot;1&quot;&gt;
&lt;tbody&gt;
&lt;tr&gt;
&lt;td&gt;
&lt;pre&gt;
public void setLineWidth(int width)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
this.width = width;
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;buildArrayOfSpaces(width);&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
private void &lt;b&gt;buildArrayOfSpaces(int width)&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
spaces = new char[width];
&lt;/pre&gt;
&lt;pre&gt;
Arrays.fill(spaces, ' ');
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
&lt;font size=&quot;3&quot;&gt;&lt;b&gt;&lt;i&gt;Run the tests: the tests pass&lt;/i&gt;&lt;/b&gt;&lt;/font&gt;
&lt;/pre&gt;
&lt;/td&gt;
&lt;/tr&gt;
&lt;/tbody&gt;
&lt;/table&gt;
&lt;p&gt;
I don't like the way &lt;font size=&quot;3&quot;&gt;&lt;tt&gt;center&lt;/tt&gt;&lt;/font&gt; function is constructed. There is math scattered all through
it. I think we can rearrange the math to make things clearer.
&lt;/p&gt;
&lt;table width=&quot;100%&quot; border=&quot;1&quot;&gt;
&lt;tbody&gt;
&lt;tr&gt;
&lt;td&gt;
&lt;pre&gt;
public String center(String line)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;int remainder = line.length() % 2;&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;int numberOfBlanksInFront = (width - line.length()) / 2;&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;int numberOfBlanksAtEnd = (width - line.length()) / 2 + remainder;&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
StringBuffer b = new StringBuffer();
&lt;/pre&gt;
&lt;pre&gt;
b.append(spaces, 0, &lt;b&gt;numberOfBlanksInFront&lt;/b&gt;);
&lt;/pre&gt;
&lt;pre&gt;
b.append(line);
&lt;/pre&gt;
&lt;pre&gt;
b.append(spaces, 0, &lt;b&gt;numberOfBlanksAtEnd&lt;/b&gt;);
&lt;/pre&gt;
&lt;pre&gt;
return b.toString();
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;font size=&quot;3&quot;&gt;&lt;b&gt;&lt;i&gt;Run the tests: the tests pass&lt;/i&gt;&lt;/b&gt;&lt;/font&gt;&lt;br /&gt;
&lt;/pre&gt;
&lt;/td&gt;
&lt;/tr&gt;
&lt;/tbody&gt;
&lt;/table&gt;
&lt;p&gt;
This looks good, but we can reduce the clutter by changing some of the variables into functions.
&lt;/p&gt;
&lt;table width=&quot;100%&quot; border=&quot;1&quot;&gt;
&lt;tbody&gt;
&lt;tr&gt;
&lt;td&gt;
&lt;pre&gt;
public String center(String line)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
StringBuffer b = new StringBuffer();
&lt;/pre&gt;
&lt;pre&gt;
b.append(spaces, 0, &lt;b&gt;numberOfBlanksInFront(line)&lt;/b&gt;);
&lt;/pre&gt;
&lt;pre&gt;
b.append(line);
&lt;/pre&gt;
&lt;pre&gt;
b.append(spaces, 0, &lt;b&gt;numberOfBlanksBehind(line)&lt;/b&gt;);
&lt;/pre&gt;
&lt;pre&gt;
return b.toString();
}
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;private int numberOfBlanksBehind(String line)&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;{&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;int extraBlankIfOdd = line.length() % 2;&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;return (width - line.length()) / 2 + extraBlankIfOdd;&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;}&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;private int numberOfBlanksInFront(String line)&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;{&lt;/b&gt;
&lt;b&gt;return (width - line.length()) / 2;&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;}&lt;/b&gt;
&lt;font size=&quot;3&quot;&gt;&lt;b&gt;&lt;i&gt;Run the tests: the tests pass&lt;/i&gt;&lt;/b&gt;&lt;/font&gt;
&lt;/pre&gt;
&lt;/td&gt;
&lt;/tr&gt;
&lt;/tbody&gt;
&lt;/table&gt;
&lt;p&gt;
This makes the &lt;font size=&quot;3&quot;&gt;&lt;tt&gt;center&lt;/tt&gt;&lt;/font&gt; function read a little better. However, the use of the &lt;font
size=&quot;3&quot;&gt;&lt;tt&gt;StringBuffer.append&lt;/tt&gt;&lt;/font&gt; function is a little confusing. We might be able to improve it a little by
creating a more explicit function.
&lt;/p&gt;
&lt;table width=&quot;100%&quot; border=&quot;1&quot;&gt;
&lt;tbody&gt;
&lt;tr&gt;
&lt;td&gt;
&lt;pre&gt;
public String center(String line)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
StringBuffer b = new StringBuffer();
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;appendBlanks(b, numberOfBlanksInFront(line));&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
b.append(line);
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt;appendBlanks(b, numberOfBlanksBehind(line));&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
return b.toString();
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
&lt;strong&gt;private void appendBlanks(StringBuffer b, int numberOfBlanks)&lt;/strong&gt;
&lt;/pre&gt;
&lt;pre&gt;
&lt;strong&gt;{&lt;/strong&gt;
&lt;/pre&gt;
&lt;pre&gt;
&lt;strong&gt;b.append(spaces, 0, numberOfBlanks);&lt;/strong&gt;
&lt;/pre&gt;
&lt;pre&gt;
&lt;strong&gt;}&lt;/strong&gt;
&lt;font size=&quot;3&quot;&gt;&lt;b&gt;&lt;i&gt;Run the tests: the tests pass&lt;/i&gt;&lt;/b&gt;&lt;/font&gt;
&lt;/pre&gt;
&lt;/td&gt;
&lt;/tr&gt;
&lt;/tbody&gt;
&lt;/table&gt;
&lt;p&gt;
Now we can rewrite &lt;font size=&quot;3&quot;&gt;&lt;tt&gt;appendBlanks&lt;/tt&gt;&lt;/font&gt; to avoid using the &lt;font size=&quot;3&quot;&gt;&lt;tt&gt;spaces&lt;/tt&gt;&lt;/font&gt;
array.
&lt;/p&gt;
&lt;table width=&quot;100%&quot; border=&quot;1&quot;&gt;
&lt;tbody&gt;
&lt;tr&gt;
&lt;td&gt;
&lt;pre&gt;
import java.util.Arrays;
&lt;/pre&gt;
&lt;pre&gt;
public class Formatter
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
private int width;
&lt;/pre&gt;
&lt;pre&gt;
public void setLineWidth(int width)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
this.width = width;
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
public String center(String line)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
StringBuffer b = new StringBuffer();
&lt;/pre&gt;
&lt;pre&gt;
appendBlanks(b, numberOfBlanksInFront(line));
&lt;/pre&gt;
&lt;pre&gt;
b.append(line);
&lt;/pre&gt;
&lt;pre&gt;
appendBlanks(b, numberOfBlanksBehind(line));
&lt;/pre&gt;
&lt;pre&gt;
return b.toString();
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
private void appendBlanks(StringBuffer b, int numberOfBlanks)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt; while(numberOfBlanks-- &amp;gt; 0)&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
&lt;b&gt; b.append(' ');&lt;/b&gt;
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
private int numberOfBlanksBehind(String line)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
int extraBlankIfOdd = line.length() % 2;
&lt;/pre&gt;
&lt;pre&gt;
return (width - line.length()) / 2 + extraBlankIfOdd;
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
private int numberOfBlanksInFront(String line)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
return (width - line.length()) / 2; }
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
}&lt;font size=&quot;3&quot;&gt;&lt;b&gt;&lt;i&gt;Run the tests: the tests pass&lt;/i&gt;&lt;/b&gt;&lt;/font&gt;&lt;br /&gt;
&lt;/pre&gt;
&lt;/td&gt;
&lt;/tr&gt;
&lt;/tbody&gt;
&lt;/table&gt;
&lt;p&gt;
The names of functions like &lt;font size=&quot;3&quot;&gt;&lt;tt&gt;numberOfBlanksBehind&lt;/tt&gt;&lt;/font&gt; imply that the reader knows that these
will be called from the &lt;font size=&quot;3&quot;&gt;&lt;tt&gt;center&lt;/tt&gt;&lt;/font&gt; function. We should eliminate this implication by
renaming those functions.
&lt;/p&gt;
&lt;table width=&quot;100%&quot; border=&quot;1&quot;&gt;
&lt;tbody&gt;
&lt;tr&gt;
&lt;td&gt;
&lt;pre&gt;
import java.util.Arrays;
&lt;/pre&gt;
&lt;pre&gt;
public class Formatter
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
private int width;
&lt;/pre&gt;
&lt;pre&gt;
public void setLineWidth(int width)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
this.width = width;
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
public String center(String line)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
StringBuffer b = new StringBuffer();
&lt;/pre&gt;
&lt;pre&gt;
appendBlanks(b, &lt;b&gt;numberOfBlanksToLeftOfCenter&lt;/b&gt;(line));
&lt;/pre&gt;
&lt;pre&gt;
b.append(line);
&lt;/pre&gt;
&lt;pre&gt;
appendBlanks(b, &lt;b&gt;numberOfBlanksToRightOfCenter&lt;/b&gt;(line));
&lt;/pre&gt;
&lt;pre&gt;
return b.toString();
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
private void appendBlanks(StringBuffer b, int numberOfBlanks)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
while(numberOfBlanks-- &amp;gt; 0)
&lt;/pre&gt;
&lt;pre&gt;
b.append(' ');
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
private int &lt;b&gt;numberOfBlanksToRightOfCenter&lt;/b&gt;(String line)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
int extraBlankIfOdd = line.length() % 2;
&lt;/pre&gt;
&lt;pre&gt;
return (width - line.length()) / 2 + extraBlankIfOdd;
&lt;/pre&gt;
&lt;pre&gt;
}
private int &lt;b&gt;numberOfBlanksToLeftOfCenter&lt;/b&gt;(String line)
&lt;/pre&gt;
&lt;pre&gt;
{
&lt;/pre&gt;
&lt;pre&gt;
return (width - line.length()) / 2;
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;/pre&gt;
&lt;pre&gt;
}
&lt;font size=&quot;3&quot;&gt;&lt;b&gt;&lt;i&gt;Run the tests: the tests pass&lt;/i&gt;&lt;/b&gt;&lt;/font&gt;
&lt;/pre&gt;
&lt;/td&gt;
&lt;/tr&gt;
&lt;/tbody&gt;
&lt;/table&gt;
&lt;p&gt;
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.
&lt;/p&gt;</mainDescription>
</org.eclipse.epf.uma:ContentDescription>