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