Refactoring Workbook - Source Code



***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Refactoring Workbook



***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Refactoring
What is Refactoring? It is improving the design of your code without adding new behavior. Why would you want to do that? To boost your productivity. How does it help do that? By helping you produce smaller, simpler, better communicating code. To learn more, read Martin Fowler's classic: Refactoring: Improving the Design of Existing Code.


Refactoring To Patterns
Download the latest draft (Adobe Acrobat, under 750k)
Last updated: February 11, 2003
Refactoring to Patterns is a growing body of work that includes neary 2 dozen refactorings. As I continue to improve and extend the book, I welcome your feedback and suggestions. I'll be presenting tutorials on this subject at upcoming conferences as well as in The Design Patterns Workshop. -- Joshua Kerievsky




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Errata for Refactoring

Here are the current known errors in the Refactoring book.

---

Many thanks to Mike Anderson, Alex Aptekman, Beth Egan Bradtke, Greg Cohoon, George Cowan, John Dale, Dion Dock, Jutta Eckstein, Paul Haahr, John Hollister, Heinz Kabutz, Bernd Kahlbrandt, Bart Koestner, Jung-joon Kim, Mark Kinzie, Hamish Lawson, Hwijae Lee, Jaeik Lee, Marc Lepage, Ron Lusk, Rory Molinari, Anthon van der Neut, Orjan Petersson, Jon Reid, Oliver Rode, Phil Rodgers, Gavin Scott, Patricia Shanahan, Pradyumn Sharma, Joel Smith, Ellen Spertus, Dawie Strauss, Frank Tip, Madhavi Tolety, Bill Wake and Hirohide Yazaki  for spotting and telling me about these errors

---

Errors in the eighth (and later) printing

Page 2:
        In the first paragraph "and identifies the type movie" should read "and identifies the type of movie".

Page 12:
        

I've been asked for more information about the double to int rounding. It happens when you have a compound assignment. In other words
        

        
        int foo=1;
        foo += 1.5;
        
        

compiles, as it is equivalent to

        
        int foo = 1;
        foo = (int) (foo + 1.5);
        
        

I haven't looked into the rationale behind this. (see Java Language Specification, section 15.26.2)
        

Page 40:
In the code examples on pages 40 and 41 the references to the field _name should instead be _title

Page 98:
        The method testReadAtEnd is incorrect. I looked at my source files and found the method there to say

        public void testReadAtEnd() throws IOException {

                int ch = -1234;

                for        (int i = 0; i < 141; i++)

                        ch = _input.read();

                assertEquals("read at end", -1, _input.read());

        }

        

        Another reason to be glad that these days I'm auto-inserting source code!

Page 115:
In the third line, "rerun" should be "return"

Page 120:
        In the solution statement the phrase "Replace all references to the temp with the expression" should be replaced with "Replace all references to the temp with the new method"

Page 121:
        This was an incorrect fix to an earlier error. In the last line of the mechanics the refactoring 'Replace Temp with Inline Temp' should read 'Inline Temp'. The same problem occurs on page 122.

Page 153:
        In the second para, third line "As discussed in Lea by section..." should read "As discussed in Lea in section...."

Page 176:
        In the second para, (a step in the mechanics) "change the getting method" should read "change the setting method"

Page 185:
        In the second paragraph "biwise xor" shoudl read "bitwise xor"

Page 222:
        At the bottom of the page I make the method getCode private. I obviously can't do that while any clients of BloodGroup are using the method.

Page 225:
        The first mention of the create method is missing the static keyword.

Page 240:
        On this page and on later pages I use the method isNotEligibleForDisability. Since my spell checker doesn't look at code it didn't tell me that the correct spelling is Eligable.

Page 301:
        There are problems with the example, see the discussion in RemoveSettingMethod.

Page 363:
        The references to the refactorings on this page are cross referenced with chapter references rather than the usual page references. Worse still, the chapter for Extract Class should be Chapter 7.

---

Errors in the fifth through eighth printing

Page 2:
        In the first paragraph "and identifies the type movie" should read "and identifies the type of movie".

Page 12:
        

I've been asked for more information about the double to int rounding. It happens when you have a compound assignment. In other words
        

        
        int foo=1;
        foo += 1.5;
        
        

compiles, as it is equivalent to

        
        int foo = 1;
        foo = (int) (foo + 1.5);
        
        

I haven't looked into the rationale behind this. (see Java Language Specification, section 15.26.2)
        

Page 25 :
        
        Caption on fig 1.7 "Sequence diagram before extraction..." should be "Sequence diagram after extraction..."
[Corrected in the 8th Printing]

Page 40:
In the code examples on pages 40 and 41 the references to the field _name should instead be _title

Page 76 :
        
        3rd para in "Duplicated Code". "in both classes then Pull Up Field (320)" should read "in both classes then Pull Up Method (322)"
[Corrected in the 8th Printing]

Page 98:
        The method testReadAtEnd is incorrect. I looked at my source files and found the method there to say

        public void testReadAtEnd() throws IOException {

                int ch = -1234;

                for        (int i = 0; i < 141; i++)

                        ch = _input.read();

                assertEquals("read at end", -1, _input.read());

        }

        

        Another reason to be glad that these days I'm auto-inserting source code!

Page 115:
In the third line, "rerun" should be "return"

Page 120:
        In the solution statement the phrase "Replace all references to the temp with the expression" should be replaced with "Replace all references to the temp with the new method"

Page 121 :
        
        In the last step of the mechanics, I say to use Replace Temp with Query (120), this should be replaced with a reference to Inline Temp (119). Otherwise I'd get a recursive refactoring. The problem continues in the example on page 122 where again the cross reference should be to Inline Temp (119). This was fixed incorrectly in the 8th printing (see below)
[Corrected in the 8th Printing]

Page 121:
        This was an incorrect fix to an earlier error. In the last line of the mechanics the refactoring 'Replace Temp with Inline Temp' should read 'Inline Temp'. The same problem occurs on page 122.

Page 153:
        In the second para, third line "As discussed in Lea by section..." should read "As discussed in Lea in section...."

Page 176:
        In the second para, (a step in the mechanics) "change the getting method" should read "change the setting method"

Page 176 :
        
        In the method numberOfOrdersFor the line that reads if (each.getCustomerName().equals(customer)) result++; should read if (each.getCustomer().equals(customer)) result++;
[Corrected in the 8th Printing]

Page 177 :
        
        In the first state of the method setCustomer the line that reads _customer = new Customer (customer); should read _customer = new Customer (arg);
[Corrected in the 8th Printing]

Page 185:
        In the second paragraph "biwise xor" shoudl read "bitwise xor"

Page 219 :
        
        "In a paragraph at around the middle of page, the sentence "...need a new method that returns the code" should read "...need a new method that returns an instance of the new class."
[Corrected in the 8th Printing]

Page 222 :
        
        "In the crossed out code, the method "public int getBloodGroup() {" should be "public int getBloodGroupCode() {" (I had just renamed it!)"
[Corrected in the 8th Printing]

Page 222:
        At the bottom of the page I make the method getCode private. I obviously can't do that while any clients of BloodGroup are using the method.

Page 225:
        The first mention of the create method is missing the static keyword.

Page 240:
        On this page and on later pages I use the method isNotEligibleForDisability. Since my spell checker doesn't look at code it didn't tell me that the correct spelling is Eligable.

Page 285 :
        
        The before code contains a rather more serious error than the refactoring is meant to help with as the assertion is always executed.
        The before code should read:
        

        void setValue (String name, int value) {

                if (name.equals("height")) {

                        _height = value;

                        return;

                }

                if (name.equals("width")) {

                        _width = value;

                        return;

                }

                Assert.shouldNeverReachHere();

        }

        

[Corrected in the 8th Printing]

Page 301:
        There are problems with the example, see the discussion in RemoveSettingMethod.

Page 313 :
        
        

The line of code that says

    
        
        Assert.isTrue("amount too large", amount > _balance);
        
        

is in error as the sense of the boolean is the wrong way round. A better line would be
        

        
        Assert.isTrue("sufficient funds", amount <= _balance);
        
[Corrected in the 8th Printing]

Page 324 :
        
        In the top code example the line double chargeAmount = charge (lastBillDate, date) should read double chargeAmount = chargeFor (lastBillDate, date). (I got the method name inconsistent with the diagrams.)
        
[Corrected in the 8th Printing]

Page 328 :
        
        In the motivation paragraph "Pull Down Method" should read "Push Down Method". (This is what happens when I don't use links for everything!)
        
[Corrected in the 8th Printing]

Page 363:
        The references to the refactorings on this page are cross referenced with chapter references rather than the usual page references. Worse still, the chapter for Extract Class should be Chapter 7.

---

Errors in the fourth printing

Page xx :
        
"Joshua suggested the idea of code sketches" should read "Joshua Kerievsky suggested the idea of code sketches"
[Corrected in the 5th Printing]

Page 2:
        In the first paragraph "and identifies the type movie" should read "and identifies the type of movie".

Page 12:
        

I've been asked for more information about the double to int rounding. It happens when you have a compound assignment. In other words
        

        
        int foo=1;
        foo += 1.5;
        
        

compiles, as it is equivalent to

        
        int foo = 1;
        foo = (int) (foo + 1.5);
        
        

I haven't looked into the rationale behind this. (see Java Language Specification, section 15.26.2)
        

Page 25 :
        
        Caption on fig 1.7 "Sequence diagram before extraction..." should be "Sequence diagram after extraction..."
[Corrected in the 8th Printing]

Page 40:
In the code examples on pages 40 and 41 the references to the field _name should instead be _title

Page 76 :
        
        3rd para in "Duplicated Code". "in both classes then Pull Up Field (320)" should read "in both classes then Pull Up Method (322)"
[Corrected in the 8th Printing]

Page 98:
        The method testReadAtEnd is incorrect. I looked at my source files and found the method there to say

        public void testReadAtEnd() throws IOException {

                int ch = -1234;

                for        (int i = 0; i < 141; i++)

                        ch = _input.read();

                assertEquals("read at end", -1, _input.read());

        }

        

        Another reason to be glad that these days I'm auto-inserting source code!

Page 115:
In the third line, "rerun" should be "return"

Page 120:
        In the solution statement the phrase "Replace all references to the temp with the expression" should be replaced with "Replace all references to the temp with the new method"

Page 121 :
        
        In the last step of the mechanics, I say to use Replace Temp with Query (120), this should be replaced with a reference to Inline Temp (119). Otherwise I'd get a recursive refactoring. The problem continues in the example on page 122 where again the cross reference should be to Inline Temp (119). This was fixed incorrectly in the 8th printing (see below)
[Corrected in the 8th Printing]

Page 121:
        This was an incorrect fix to an earlier error. In the last line of the mechanics the refactoring 'Replace Temp with Inline Temp' should read 'Inline Temp'. The same problem occurs on page 122.

Page 153:
        In the second para, third line "As discussed in Lea by section..." should read "As discussed in Lea in section...."

Page 176:
        In the second para, (a step in the mechanics) "change the getting method" should read "change the setting method"

Page 176 :
        
        In the method numberOfOrdersFor the line that reads if (each.getCustomerName().equals(customer)) result++; should read if (each.getCustomer().equals(customer)) result++;
[Corrected in the 8th Printing]

Page 177 :
        
        In the first state of the method setCustomer the line that reads _customer = new Customer (customer); should read _customer = new Customer (arg);
[Corrected in the 8th Printing]

Page 185:
        In the second paragraph "biwise xor" shoudl read "bitwise xor"

Page 193 :
        
"to declare that interval window implements Observable" should read  "to declare that interval window implements Observer" [Corrected in the 5th Printing]

Page 219 :
        
        "In a paragraph at around the middle of page, the sentence "...need a new method that returns the code" should read "...need a new method that returns an instance of the new class."
[Corrected in the 8th Printing]

Page 222 :
        
        "In the crossed out code, the method "public int getBloodGroup() {" should be "public int getBloodGroupCode() {" (I had just renamed it!)"
[Corrected in the 8th Printing]

Page 222:
        At the bottom of the page I make the method getCode private. I obviously can't do that while any clients of BloodGroup are using the method.

Page 225:
        The first mention of the create method is missing the static keyword.

Page 240:
        On this page and on later pages I use the method isNotEligibleForDisability. Since my spell checker doesn't look at code it didn't tell me that the correct spelling is Eligable.

Page 261 :
        
In Ron's story, 4th para, "Of course, as soon as you being inspecting..." but should be "Of course, as soon as you begin
inspecting..."
  [Corrected in the 5th Printing]

Page 285 :
        
        The before code contains a rather more serious error than the refactoring is meant to help with as the assertion is always executed.
        The before code should read:
        

        void setValue (String name, int value) {

                if (name.equals("height")) {

                        _height = value;

                        return;

                }

                if (name.equals("width")) {

                        _width = value;

                        return;

                }

                Assert.shouldNeverReachHere();

        }

        

[Corrected in the 8th Printing]

Page 300 :
        
        In the mechanics section the field should be made final at the end
        of process not at the begining.
[Corrected in the 5th Printing]

Page 301:
        There are problems with the example, see the discussion in RemoveSettingMethod.

Page 307 :
        
"Another reason to be wary of class.forName is that..."
should be:
"Another reason to be wary of Class.forName is that..." (Class should have a capital C)
[Corrected in the 5th Printing]

Page 307 :
        
"I can use a differenct approach...."
should be:
"I can use a different approach..." (spelling) [Corrected in the 5th Printing]

Page 311 :
        
"If the exception us checked, adjust the callers..."
should be:
"If the exception is checked, adjust the callers..."
[Corrected in the 5th Printing]

Page 313 :
        
        

The line of code that says

    
        
        Assert.isTrue("amount too large", amount > _balance);
        
        

is in error as the sense of the boolean is the wrong way round. A better line would be
        

        
        Assert.isTrue("sufficient funds", amount <= _balance);
        
[Corrected in the 8th Printing]

Page 324 :
        
        In the top code example the line double chargeAmount = charge (lastBillDate, date) should read double chargeAmount = chargeFor (lastBillDate, date). (I got the method name inconsistent with the diagrams.)
        
[Corrected in the 8th Printing]

Page 328 :
        
        In the motivation paragraph "Pull Down Method" should read "Push Down Method". (This is what happens when I don't use links for everything!)
        
[Corrected in the 8th Printing]

Page 333 :
        
"arguments are needed by the labor item, and some are not"
should be: "some arguments are needed..." (missing word) [Corrected in the 5th Printing]

Page 346 :
        
"Whenever we see two similar method" should be: "Whenever we
see two similar methods" (plural) [Corrected in the 5th Printing]

Page 346 :
        
"The statement method prints statements" should be: "The
statement method prints statements" (font) [Corrected in the 5th Printing]

Page 363:
        The references to the refactorings on this page are cross referenced with chapter references rather than the usual page references. Worse still, the chapter for Extract Class should be Chapter 7.

---

Errors in the third printing

Page xx :
        
"Joshua suggested the idea of code sketches" should read "Joshua Kerievsky suggested the idea of code sketches"
[Corrected in the 5th Printing]

Page 2:
        In the first paragraph "and identifies the type movie" should read "and identifies the type of movie".

Page 12:
        

I've been asked for more information about the double to int rounding. It happens when you have a compound assignment. In other words
        

        
        int foo=1;
        foo += 1.5;
        
        

compiles, as it is equivalent to

        
        int foo = 1;
        foo = (int) (foo + 1.5);
        
        

I haven't looked into the rationale behind this. (see Java Language Specification, section 15.26.2)
        

Page 25 :
        
        Caption on fig 1.7 "Sequence diagram before extraction..." should be "Sequence diagram after extraction..."
[Corrected in the 8th Printing]

Page 40:
In the code examples on pages 40 and 41 the references to the field _name should instead be _title

Page 76 :
        
        3rd para in "Duplicated Code". "in both classes then Pull Up Field (320)" should read "in both classes then Pull Up Method (322)"
[Corrected in the 8th Printing]

Page 98:
        The method testReadAtEnd is incorrect. I looked at my source files and found the method there to say

        public void testReadAtEnd() throws IOException {

                int ch = -1234;

                for        (int i = 0; i < 141; i++)

                        ch = _input.read();

                assertEquals("read at end", -1, _input.read());

        }

        

        Another reason to be glad that these days I'm auto-inserting source code!

Page 115:
In the third line, "rerun" should be "return"

Page 120:
        In the solution statement the phrase "Replace all references to the temp with the expression" should be replaced with "Replace all references to the temp with the new method"

Page 121 :
        
        In the last step of the mechanics, I say to use Replace Temp with Query (120), this should be replaced with a reference to Inline Temp (119). Otherwise I'd get a recursive refactoring. The problem continues in the example on page 122 where again the cross reference should be to Inline Temp (119). This was fixed incorrectly in the 8th printing (see below)
[Corrected in the 8th Printing]

Page 121:
        This was an incorrect fix to an earlier error. In the last line of the mechanics the refactoring 'Replace Temp with Inline Temp' should read 'Inline Temp'. The same problem occurs on page 122.

Page 153:
        In the second para, third line "As discussed in Lea by section..." should read "As discussed in Lea in section...."

Page 176:
        In the second para, (a step in the mechanics) "change the getting method" should read "change the setting method"

Page 176 :
        
        In the method numberOfOrdersFor the line that reads if (each.getCustomerName().equals(customer)) result++; should read if (each.getCustomer().equals(customer)) result++;
[Corrected in the 8th Printing]

Page 177 :
        
        In the first state of the method setCustomer the line that reads _customer = new Customer (customer); should read _customer = new Customer (arg);
[Corrected in the 8th Printing]

Page 185:
        In the second paragraph "biwise xor" shoudl read "bitwise xor"

Page 193 :
        
"to declare that interval window implements Observable" should read  "to declare that interval window implements Observer" [Corrected in the 5th Printing]

Page 219 :
        
        "In a paragraph at around the middle of page, the sentence "...need a new method that returns the code" should read "...need a new method that returns an instance of the new class."
[Corrected in the 8th Printing]

Page 222 :
        
        "In the crossed out code, the method "public int getBloodGroup() {" should be "public int getBloodGroupCode() {" (I had just renamed it!)"
[Corrected in the 8th Printing]

Page 222:
        At the bottom of the page I make the method getCode private. I obviously can't do that while any clients of BloodGroup are using the method.

Page 225:
        The first mention of the create method is missing the static keyword.

Page 240:
        On this page and on later pages I use the method isNotEligibleForDisability. Since my spell checker doesn't look at code it didn't tell me that the correct spelling is Eligable.

Page 241 :
        
In the code examples at the bottom of the page, the method isEligibleForDisability
should be isNotEligibleForDisability [Corrected in the 4th Printing]

Page 261 :
        
In Ron's story, 4th para, "Of course, as soon as you being inspecting..." but should be "Of course, as soon as you begin
inspecting..."
  [Corrected in the 5th Printing]

Page 285 :
        
        The before code contains a rather more serious error than the refactoring is meant to help with as the assertion is always executed.
        The before code should read:
        

        void setValue (String name, int value) {

                if (name.equals("height")) {

                        _height = value;

                        return;

                }

                if (name.equals("width")) {

                        _width = value;

                        return;

                }

                Assert.shouldNeverReachHere();

        }

        

[Corrected in the 8th Printing]

Page 300 :
        
        In the mechanics section the field should be made final at the end
        of process not at the begining.
[Corrected in the 5th Printing]

Page 301:
        There are problems with the example, see the discussion in RemoveSettingMethod.

Page 307 :
        
"Another reason to be wary of class.forName is that..."
should be:
"Another reason to be wary of Class.forName is that..." (Class should have a capital C)
[Corrected in the 5th Printing]

Page 307 :
        
"I can use a differenct approach...."
should be:
"I can use a different approach..." (spelling) [Corrected in the 5th Printing]

Page 311 :
        
"If the exception us checked, adjust the callers..."
should be:
"If the exception is checked, adjust the callers..."
[Corrected in the 5th Printing]

Page 313 :
        
        

The line of code that says

    
        
        Assert.isTrue("amount too large", amount > _balance);
        
        

is in error as the sense of the boolean is the wrong way round. A better line would be
        

        
        Assert.isTrue("sufficient funds", amount <= _balance);
        
[Corrected in the 8th Printing]

Page 315 :
        
        The problem statement should read "You are throwing an exception
        on a condition the caller could have checked first" (the refactoring applies to all exceptions, not just checked ones)
[Corrected in the 4th Printing]

Page 324 :
        
        In the top code example the line double chargeAmount = charge (lastBillDate, date) should read double chargeAmount = chargeFor (lastBillDate, date). (I got the method name inconsistent with the diagrams.)
        
[Corrected in the 8th Printing]

Page 328 :
        
        In the motivation paragraph "Pull Down Method" should read "Push Down Method". (This is what happens when I don't use links for everything!)
        
[Corrected in the 8th Printing]

Page 333 :
        
"arguments are needed by the labor item, and some are not"
should be: "some arguments are needed..." (missing word) [Corrected in the 5th Printing]

Page 346 :
        
"Whenever we see two similar method" should be: "Whenever we
see two similar methods" (plural) [Corrected in the 5th Printing]

Page 346 :
        
"The statement method prints statements" should be: "The
statement method prints statements" (font) [Corrected in the 5th Printing]

Page 363:
        The references to the refactorings on this page are cross referenced with chapter references rather than the usual page references. Worse still, the chapter for Extract Class should be Chapter 7.

---

Errors in the first and second printings

Page xx :
        
"Joshua suggested the idea of code sketches" should read "Joshua Kerievsky suggested the idea of code sketches"
[Corrected in the 5th Printing]

Page 2:
        In the first paragraph "and identifies the type movie" should read "and identifies the type of movie".

Page 12:
        

I've been asked for more information about the double to int rounding. It happens when you have a compound assignment. In other words
        

        
        int foo=1;
        foo += 1.5;
        
        

compiles, as it is equivalent to

        
        int foo = 1;
        foo = (int) (foo + 1.5);
        
        

I haven't looked into the rationale behind this. (see Java Language Specification, section 15.26.2)
        

Page 25 :
        
        Caption on fig 1.7 "Sequence diagram before extraction..." should be "Sequence diagram after extraction..."
[Corrected in the 8th Printing]

Page 37 :
        
"Class rental" should be "class Rental" (capitalization) and "class
movie" should be "class Movie" (capitalization)
[Corrected in the 3rd Printing]

Page 40:
In the code examples on pages 40 and 41 the references to the field _name should instead be _title

Page 48 :
        
The second line: "class Rental..." should be: "class Movie..." [Corrected in the 3rd Printing]

Page 70 :
        
Steve McConnell's last name is misspelled in two places. [Corrected in the 3rd Printing]

Page 76 :
        
        3rd para in "Duplicated Code". "in both classes then Pull Up Field (320)" should read "in both classes then Pull Up Method (322)"
[Corrected in the 8th Printing]

Page 82 :
        
        The sentence "If you add a new clause to the switch, you have to find all these switch, statements and change them." The second comma should be removed.
[Corrected in the 3rd Printing]

Page 85 :
        
"Replace Delegation with Inheritance (355)" in Inappropriate Intimacy should be "Replace Inheritance with Delegation (352)"  [Corrected in the 3rd Printing]

Page 92 :
        
        On Figure 4.1 the line from TestSuite to Test should be an association
        not a generalization (see diagram below). Also the
        package name should be junit.framework.
[Corrected in the 3rd Printing]

Page 92 :
        
        In the test file for the example, George Headley's career total was actually 2190 test riuns.
[Corrected in the 3rd Printing]

Page 98:
        The method testReadAtEnd is incorrect. I looked at my source files and found the method there to say

        public void testReadAtEnd() throws IOException {

                int ch = -1234;

                for        (int i = 0; i < 141; i++)

                        ch = _input.read();

                assertEquals("read at end", -1, _input.read());

        }

        

        Another reason to be glad that these days I'm auto-inserting source code!

Page 115 :
        
In the second sentence, "oustanding" should be "outstanding"
[Corrected in the 3rd Printing]

Page 115:
In the third line, "rerun" should be "return"

Page 120:
        In the solution statement the phrase "Replace all references to the temp with the expression" should be replaced with "Replace all references to the temp with the new method"

Page 121 :
        
        In the last step of the mechanics, I say to use Replace Temp with Query (120), this should be replaced with a reference to Inline Temp (119). Otherwise I'd get a recursive refactoring. The problem continues in the example on page 122 where again the cross reference should be to Inline Temp (119). This was fixed incorrectly in the 8th printing (see below)
[Corrected in the 8th Printing]

Page 121:
        This was an incorrect fix to an earlier error. In the last line of the mechanics the refactoring 'Replace Temp with Inline Temp' should read 'Inline Temp'. The same problem occurs on page 122.

Page 153:
        In the second para, third line "As discussed in Lea by section..." should read "As discussed in Lea in section...."

Page 176:
        In the second para, (a step in the mechanics) "change the getting method" should read "change the setting method"

Page 176 :
        
        In the method numberOfOrdersFor the line that reads if (each.getCustomerName().equals(customer)) result++; should read if (each.getCustomer().equals(customer)) result++;
[Corrected in the 8th Printing]

Page 177 :
        
        In the first state of the method setCustomer the line that reads _customer = new Customer (customer); should read _customer = new Customer (arg);
[Corrected in the 8th Printing]

Page 185:
        In the second paragraph "biwise xor" shoudl read "bitwise xor"

Page 193 :
        
"to declare that interval window implements Observable" should read  "to declare that interval window implements Observer" [Corrected in the 5th Printing]

Page 219 :
        
        "In a paragraph at around the middle of page, the sentence "...need a new method that returns the code" should read "...need a new method that returns an instance of the new class."
[Corrected in the 8th Printing]

Page 222 :
        
        "In the crossed out code, the method "public int getBloodGroup() {" should be "public int getBloodGroupCode() {" (I had just renamed it!)"
[Corrected in the 8th Printing]

Page 222:
        At the bottom of the page I make the method getCode private. I obviously can't do that while any clients of BloodGroup are using the method.

Page 225:
        The first mention of the create method is missing the static keyword.

Page 240:
        On this page and on later pages I use the method isNotEligibleForDisability. Since my spell checker doesn't look at code it didn't tell me that the correct spelling is Eligable.

Page 241 :
        
In the code examples at the bottom of the page, the method isEligibleForDisability
should be isNotEligibleForDisability [Corrected in the 4th Printing]

Page 261 :
        
In Ron's story, 4th para, "Of course, as soon as you being inspecting..." but should be "Of course, as soon as you begin
inspecting..."
  [Corrected in the 5th Printing]

Page 285 :
        
        The before code contains a rather more serious error than the refactoring is meant to help with as the assertion is always executed.
        The before code should read:
        

        void setValue (String name, int value) {

                if (name.equals("height")) {

                        _height = value;

                        return;

                }

                if (name.equals("width")) {

                        _width = value;

                        return;

                }

                Assert.shouldNeverReachHere();

        }

        

[Corrected in the 8th Printing]

Page 300 :
        
        In the mechanics section the field should be made final at the end
        of process not at the begining.
[Corrected in the 5th Printing]

Page 301:
        There are problems with the example, see the discussion in RemoveSettingMethod.

Page 307 :
        
"Another reason to be wary of class.forName is that..."
should be:
"Another reason to be wary of Class.forName is that..." (Class should have a capital C)
[Corrected in the 5th Printing]

Page 307 :
        
"I can use a differenct approach...."
should be:
"I can use a different approach..." (spelling) [Corrected in the 5th Printing]

Page 311 :
        
"If the exception us checked, adjust the callers..."
should be:
"If the exception is checked, adjust the callers..."
[Corrected in the 5th Printing]

Page 313 :
        
        

The line of code that says

    
        
        Assert.isTrue("amount too large", amount > _balance);
        
        

is in error as the sense of the boolean is the wrong way round. A better line would be
        

        
        Assert.isTrue("sufficient funds", amount <= _balance);
        
[Corrected in the 8th Printing]

Page 315 :
        
        The problem statement should read "You are throwing an exception
        on a condition the caller could have checked first" (the refactoring applies to all exceptions, not just checked ones)
[Corrected in the 4th Printing]

Page 324 :
        
        In the top code example the line double chargeAmount = charge (lastBillDate, date) should read double chargeAmount = chargeFor (lastBillDate, date). (I got the method name inconsistent with the diagrams.)
        
[Corrected in the 8th Printing]

Page 328 :
        
        In the motivation paragraph "Pull Down Method" should read "Push Down Method". (This is what happens when I don't use links for everything!)
        
[Corrected in the 8th Printing]

Page 333 :
        
"arguments are needed by the labor item, and some are not"
should be: "some arguments are needed..." (missing word) [Corrected in the 5th Printing]

Page 346 :
        
"Whenever we see two similar method" should be: "Whenever we
see two similar methods" (plural) [Corrected in the 5th Printing]

Page 346 :
        
"The statement method prints statements" should be: "The
statement method prints statements" (font) [Corrected in the 5th Printing]

Page 355 :
        
first line of section "Motivation", "...Replace Delegation with Inheritance (355)" should read "Replace Inheritance with Delegation (352)"  [Corrected in the 3rd Printing]

Page 363:
        The references to the refactorings on this page are cross referenced with chapter references rather than the usual page references. Worse still, the chapter for Extract Class should be Chapter 7.

Page 390 :
        
"...vivc.edu" should be "uiuc.edu"  [Corrected in the 3rd Printing]

Page 405 :
        
Parse tree for program should have "hello" (method name) in lower-case.
[Corrected in the 3rd Printing]

Page 405 :
        
Last box on bottom right should have "Hello World" (not "out")  [Corrected in the 3rd Printing]

Page 414 :
        
Reference to JUnit in the URL "compuserv" should be "compuserve"
[Corrected in the 3rd Printing]

Corrected version of fig 4.1 on page 92.

***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Wrap entities with session

Entity beans from the business tier are exposed to clients in another tier

Use a Session Facade to encapsulate the entity beans

For further information see page 104 of Core J2EE Patterns by Alur, Crupi, and Malks




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Use a Connection Pool

Database connections are not shared. Instead, clients manage their own connections for making database invocations

Use a connection pool to pre-initialize multiple connections, improving scalability and performance

For further information see page 119 of Core J2EE Patterns by Alur, Crupi, and Malks




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Substitute Algorithm

You want to replace an algorithm with one that is clearer.

Replace the body of the method with the new algorithm.

        String foundPerson(String[] people){
                for (int i = 0; i < people.length; i++) {
                        if (people[i].equals ("Don")){
                                return "Don";
                        }
                        if (people[i].equals ("John")){
                                return "John";
                        }
                        if (people[i].equals ("Kent")){
                                return "Kent";
                        }
                }
                return "";
        }



        String foundPerson(String[] people){
                List candidates = Arrays.asList(new String[] {"Don", "John", "Kent"});
                for (int i=0; i<people.length; i++)
                        if (candidates.contains(people[i]))
                                return people[i];
                return "";
        }


For more information see page 139 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Split Temporary Variable

You have a temporary variable assigned to more than once, but is not a loop variable nor a collecting temporary variable.

Make a separate temporary variable for each assignment.

                double temp = 2 * (_height + _width);
                System.out.println (temp);
                temp = _height * _width;
                System.out.println (temp);



                final double perimeter = 2 * (_height + _width);
                System.out.println (perimeter);
                final double area = _height * _width;
                System.out.println (area);


For more information see page 128 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Split Loop

Refactoring contributed by Martin Fowler

You have a loop that is doing two things

Duplicate the loop

void printValues() {
        double averageAge = 0;
        double totalSalary = 0;
        for (int i = 0; i < people.length; i++) {
                        averageAge += people[i].age;
                        totalSalary += people[i].salary;
        }
        averageAge = averageAge / people.length;
        System.out.println(averageAge);
        System.out.println(totalSalary);
}

void printValues() {
        double totalSalary = 0;
        for (int i = 0; i < people.length; i++) {
                        totalSalary += people[i].salary;
        }

        double averageAge = 0;
        for (int i = 0; i < people.length; i++) {
                        averageAge += people[i].age;
        }
        averageAge = averageAge / people.length;

        System.out.println(averageAge);
        System.out.println(totalSalary);
}

Motivation

You often see loops that are doing two different things at once, because they can do that with one pass through a loop. Indeed most programmers would feel very uncomfortable with this refactoring as it forces you to execute the loop twice - which is double the work.

But like so many optimizations, doing two different things in one loop is less clear than doing them separately. It also causes problems for further refactoring as it introduces temps that get in the way of further refactorings. So while refactoring, don't be afraid to get rid of the loop. When you optimize, if the loop is slow that will show up and it would be right to slam the loops back together at that point. You may be surprised at how often the loop isn't a bottleneck, or how the later refactorings open up another, more powerful, optimization.

Mechanics

• Copy the loop and remove the differing pieces from each loop


• Compile and test.


• Reorganize the lines to group the loop with related code from outside the loop


• Compile and test.


• Consider applying
  Extract Method or
  Replace Temp with Query on each loop

Example

Start with this code.


        private Person [] people;

        void printValues() {
                double averageAge = 0;
                double totalSalary = 0;
                for (int i = 0; i < people.length; i++) {
                                averageAge += people[i].age;
                                totalSalary += people[i].salary;
                }
                averageAge = averageAge / people.length;
                System.out.println(averageAge);
                System.out.println(totalSalary);
        }

The loop is calculating both the total salary and the average age. This is two separate tasks, which just happen to use the same data structure. So I'll split the loops for now, knowing I can optimize later.

The first move is to copy the loop and remove from each leg one part of the calculation.

        void printValues() {
                double averageAge = 0;
                double totalSalary = 0;
                for (int i = 0; i < people.length; i++) {
                                totalSalary += people[i].salary;
                }
                for (int i = 0; i < people.length; i++) {
                                averageAge += people[i].age;
                }
                averageAge = averageAge / people.length;
                System.out.println(averageAge);
                System.out.println(totalSalary);
        }

I can now compile and test this.

Then I can reorganize the text to group things together.


        void printValues() {
                double totalSalary = 0;
                for (int i = 0; i < people.length; i++) {
                                totalSalary += people[i].salary;
                }

                double averageAge = 0;
                for (int i = 0; i < people.length; i++) {
                                averageAge += people[i].age;
                }
                averageAge = averageAge / people.length;

                System.out.println(averageAge);
                System.out.println(totalSalary);
        }


One thing that has bothered me about this refactoring is that there's a lot of duplicate loop setup code - and duplication is the worst smell of all. On the whole, however, I don't feel that concerned about it. Fundamentally the duplication is due to the godawful way that Java does loops. Other modern languages with a foreach statement avoid the duplication completely.


Officialy that's the end of the refactoring. But the important thing here is what it leads you to do. In this case I'm inclined to apply Replace Temp with Query to each loop.


        void printValues() {
                System.out.println(averageAge());
                System.out.println(totalSalary());
        }

        private double averageAge() {
                double result = 0;
                for (int i = 0; i < people.length; i++) {
                                result += people[i].age;
                }
                return result / people.length;
        }

        private double totalSalary() {
                double result = 0;
                for (int i = 0; i < people.length; i++) {
                                result += people[i].salary;
                }
                return result;
        }

Additional Comments

The split loop is an often used performance
optimisation in data intensive applications. When you are accessing to
separate arrays in the same loop you can get hit badly by the cache
misses. That is, if the arrays are large enough you loose locality of
reference and the cache fails to do the job. Put enough code here and
every operation will not hit the cache and instead have to go back out
to main memory.


By splitting loops up into small separate components that only act on
one array you get significant performance increases. In rendering code
I've seen up to an order of magnitude difference. This is particularly
beneficial if it is primitive type based and less so if class reference
or pointer based where you need an additional dereference off into a
random memory location. This technique is also very similar to loop
unravelling for performance gains (although something I doubt would ever
appear in a refactoring/patterns/OO type book :)

--Justin Couch

Thanks to Bob Bowman for spotting and fixing an error.



***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Separate Query from Modifier

You have a method that returns a value but also changes the state of an object.

Create two methods, one for the query and one for the modification.



For more information see page 279 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Separate Data Access Code

Data access code is embedded directly within a class that has other unrelated responsibilities

Extract the data access code into a new class and move the new class logically and/or physically closer to the data source


For further information see page 113 of Core J2EE Patterns by Alur, Crupi, and Malks




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Self Encapsulate Field

You are accessing a field directly, but the coupling to the field is becoming awkward.

Create getting and setting methods for the field and use only those to access the field.


        private int _low, _high;
        boolean includes (int arg) {
                return arg >= _low && arg <= _high;
        }


        private int _low, _high;
        boolean includes (int arg) {
                return arg >= getLow() && arg <= getHigh();
        }
        int getLow() {return _low;}
        int getHigh() {return _high;}


For more information see page 171 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Reverse Conditional

Refactoring contributed by Bill Murphy and Martin Fowler

You have a conditional that would be easier to understand if you reversed its sense.

Reverse the sense of the conditional and reorder the conditional's clauses.


if ( !isSummer( date ) )
  charge = winterCharge( quantity );
else
  charge = summerCharge( quantity );


if ( isSummer( date ) )
  charge = summerCharge( quantity );
else
  charge = winterCharge( quantity );

Motivation

Often conditionals can be phrased in way that makes them hard to read. This is particularly the case when they have a not in front of them. If the conditional only has a "then" clause and no "else" clause, this is reasonable. However if the conditional has both clauses, then you might as well reverse the conditional.

Mechanics

• Remove negative from conditional.


• Switch clauses


• Compile and test.

There's further discussion on this kind of refactoring on the wiki.




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Type Code with Subclasses

You have an immutable type code that affects the behavior of a class.

Replace the type code with subclasses.



For more information see page 223 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Type Code with State/Strategy

You have a type code that affects the behavior of a class, but you cannot use subclassing.

Replace the type code with a state object.

        

For more information see page 227 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Type Code with Class

A class has a numeric type code that does not affect its behavior.

Replace the number with a new class.

For more information see page 218 of Refactoring

Corrections

Privitizing the accessors for the type code

At the end of the refactoring I said that you could make those methods that use the type code, eg getCode(), private. I neglected to say that you first have to find the callers of those methods and change them to no longer use the code number.

Contributors

• Randy Coulman

***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Temp with Query

You are using a temporary variable to hold the result of an expression.

Extract the expression into a method. Replace all references to the temp with the expression. The new method can then be used in other methods.

                double basePrice = _quantity * _itemPrice;
                if (basePrice > 1000)
                        return basePrice * 0.95;
                else
                        return basePrice * 0.98;


                if (basePrice() > 1000)
                        return basePrice() * 0.95;
                else
                        return basePrice() * 0.98;
...
        double basePrice() {
                return _quantity * _itemPrice;
        }


For more information see page 120 of Refactoring

Additional Comments

Side Effects

Paul Haahr pointed out that you can't do this refactoring if the code in between the the assignment to the temp and the use of the temp changes the value of the expression that calculates the temp. In these cases the code is using the temp to snapshot the value of the temp when it's assigned. The name of the temp should convey this fact (and you should change the name if it doesn't).

He also pointed out that it is easy to forget that creating a reference object is a side effect, while creating a value object isn't.

Contributors

• Paul Haahr

***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Subclass with Fields

You have subclasses that vary only in methods that return constant data.

Change the methods to superclass fields and eliminate the subclasses.

        

For more information see page 232 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Static Variable with Parameter

Refactoring contributed by Marian Vittek


A function depending on a static variable needs to be reused in more general context.

Add a new parameter to the function and replace all references of the static variable within the function by this new parameter.


void printValues() {
        for (int i = 0; i < people.length; i++) {
                System.out.println(people[i].name+" has salary "+people[i].salary);
        }
}

public static void main(String args[]) {
        ...
        printValues();
}




void printValues(PrintStream outfile) {
        for (int i = 0; i < people.length; i++) {
                outfile.println(people[i].name+" has salary "+people[i].salary);
        }
}

public static void main(String args[]) {
        ...
        printValues(System.out);
}

Motivation

The original function is using a static variable, but you wish either to reuse the function in new project (not containing the static variable) or reuse the function in the same project but in more general context.

Mechanics

• If the function calls other functions using the static variable in question, then use this refactoring on all those invoked functions first.


• Use Add Parameter to add a new argument to the function


• Add the static variable as actual argument to all callers of this function in.


• Replace all references to the static variable within the function by the new argument

***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Recursion with Iteration

Refactoring contributed by Ivan Mitrovic
        
You have code that uses Recursion and is hard to understand.

Replace Recursion with Iteration.
                        
                                
public void countDown(int n) {
        if(n == 0) return;

        System.out.println(n + "...");
        waitASecond();
        countDown(n-1);
}
                                        

                
After Refactoring:
                                                
public void countDown(int n) {
        while(n > 0) {
                System.out.println(n + "...");
                waitASecond ();
                n -= 1;
        }
                                        

Motivation

                
The old recruiting practice says, "Never hire a developer who computes the factorial using Recursion". Recursion is often used without considering alternatives before using it. Though it is true that recursive solution is often more elegant and easier to spot than the iterative solution, one should take care not to abuse it. Complex Recursion that is hard to understand should probably be considered a "bad smell" in the code and a good candidate to be replaced with Iteration (usually in combination with some other Refactorings). Moreover, iterative solutions are usually more efficient than recursive solutions as they don't incur the overhead of the multiple method calls.
                
We use Recursion when we have to perform a complex task that can be broken into the several subtasks. Recursion is implemented as a method that calls itself to solve subtasks. During the recursive call the values of the local fields of the method are placed on the method stack until the subtask performed by a recursive call is completed. Thus, whenever recursive method is called, local fields are put on the method stack and used again after the recursive call is completed. The general approach to Refactoring could probably be to implement the alternative Stack that "simulates" the method stack where the local fields could be placed (the java.util.Stack class is a good candidate for this job).

Sometimes a recursive solution doesn't preserve any local fields during the recursive call, which makes Refactoring much easier. This is the case with, so called, tail recursions. Tail recursions are recursions where the recursive call is the last line in the method. Tail recursions are generally considered a bad practice and should be replaced with Iteration. This technique is well known to the people who work on compiler implementations. A good compiler usually perfomes this Refactoring on the fly (this is the earliest known example that machines adopted some XP practices :)
        

Mechanics

• Determine the base case of the Recursion. Base case, when reached, causes Recursion to end. Every Recursion must have a defined base case. In addition, each recursive call must make a progress towards the base case (otherwise recursive calls would be performed infinitely). In our example the base case is n == 0.


• Implement a loop that will iterate until the base case is reached.


• Make a progress towards the base case. Send the new arguments to the top of the loop instead to the recursive method.

                
The mechanics of some complicated Refactorings other than tail recursion Refactorings (for example, those that would use "home made" Stack to store the local fields of the method) are waiting to be defined. In the meantime, if you find yourself dealing with the particularly nasty recursion, don't forget that
Substitute Algorithm is a valid Refactoring and a secret weapon when it comes to situations like this.

        

Example

Sputnik launching countdown is a simple example of tail recursion:

public class CountDown {
        public void countDown(int n) {
                if(n == 0) return;

                System.out.println(n + "...");
                waitASecond();
                countDown(n-1);
        }

        public void waitASecond() {
                try {
                        Thread.sleep(1000);
                }
                catch (InterruptedException ignore) {
                }
        }

        public static void main(String[] args) {
                CountDown c = new CountDown();
                c.countDown(10);
        }
}
                                
                
After Refactoring:

                                                
public class CountDown {
        public void countDown(int n) {
                while(n > 0) {
                        System.out.println(n + "...");
                        waitASecond ();
                        n -= 1;
                }

        }

        public void waitASecond() {
                try {
                        Thread.sleep(1000);
                }
                catch (InterruptedException ignore) {
                }
        }

        public static void main(String[] args) {
                CountDown c = new CountDown();
                c.countDown(10);
        }
}

***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Record with Data Class

You need to interface with a record structure in a traditional programming environment.x

Make a dumb data object for the record.


For more information see page 217 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Parameter with Method

An object invokes a method, then passes the result as a parameter for a method. The receiver can also invoke this method.

Remove the parameter and let the receiver invoke the method.


                int basePrice = _quantity * _itemPrice;
                discountLevel = getDiscountLevel();
                double finalPrice = discountedPrice (basePrice, discountLevel);



                int basePrice = _quantity * _itemPrice;
                double finalPrice = discountedPrice (basePrice);


For more information see page 292 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Parameter with Explicit Methods

You have a method that runs different code depending on the values of an enumerated parameter.

Create a separate method for each value of the parameter.

        void setValue (String name, int value) {
                if (name.equals("height")) {
                        _height = value;
                        return;
                }
                if (name.equals("width")) {
                        _width = value;
                        return;
                }
                Assert.shouldNeverReachHere();
        }


        void setHeight(int arg) {
                _height = arg;
        }
        void setWidth (int arg) {
                _width = arg;
        }


For more information see page 285 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Nested Conditional with Guard Clauses

A method has conditional behavior that does not make clear what the normal path of execution is

Use Guard Clauses for all the special cases

double getPayAmount() {
        double result;
        if (_isDead) result = deadAmount();
        else {
                if (_isSeparated) result = separatedAmount();
                else {
                        if (_isRetired) result = retiredAmount();
                        else result = normalPayAmount();
                };
        }
return result;
};        



double getPayAmount() {
        if (_isDead) return deadAmount();
        if (_isSeparated) return separatedAmount();
        if (_isRetired) return retiredAmount();
        return normalPayAmount();
};        


For more information see page 250 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Method with Method Object

You have a long method that uses local variables in such a way that you cannot apply
Extract Method

Turn the method into its own object so that all the local variables become fields on that object. You can then decompose the method into other methods on the same object.

class Order...
        double price() {
                double primaryBasePrice;
                double secondaryBasePrice;
                double tertiaryBasePrice;
                // long computation;
                ...
        }



        

Additional Comments

Using a Static Method

You can do this refactoring with a static method, but in this case you don't need the first field pointing back to the original object.

Alternative Steps with Temps

Marnix Klooster suggested an alternative to the mechanics. Rather than creating fields in the Method Object for all the temps, leave the temps as temps and only turn them into fields as you need to in order to use  Extract Method. It means you have to use a special form of Extract Method, but may help in reducing the scope of the temps. Of course you can use Reduce Scope of Variable afterwards on any temp that's only used in one method.

Contributors

• Marnix Klooster

For more information see page 135 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Magic Number with Symbolic Constant

You have a literal number with a particular meaning.

Create a constant, name it after the meaning, and replace the number with it.


        double potentialEnergy(double mass, double height) {
                return mass * height * 9.81;
        }


        double potentialEnergy(double mass, double height) {
                return mass * GRAVITATIONAL_CONSTANT * height;
        }
        static final double GRAVITATIONAL_CONSTANT = 9.81;


For more information see page 204 of Refactoring

Additional Comments

Using a constant method

For this refactoring I used a symbolic constant, which is the most common Java idiom.

However an alternative is the constant method, which is a method of this form

public static double gravitationalConstant() {
        return 9.81;
}

This idiom is less familiar to C based programmers, but is very familiar to Smalltalkers (who didn't have constants in their language). On the whole I don't tend to use this in Java as it is less idiomatic to the language. However if you need to replace the simple return with a calculated value then it's worth changing the constant field to a constant method. (I guess there should be a refactoring for that....)

Contributors

• Jutta Eckstein

***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Iteration with Recursion

Refactoring contributed by Dave Whipp
  
You have a loop, and it is not obvious what each iteration is
doing
  
Replace Iteration with Recursion
  


unsigned greatest_common_divisor (unsigned a, unsigned b)
{
  while (a != b)
  {
    if (a > b)
    {
      a -= b;
    }
    else if (b > a)
    {
      b -= a;
    }
  }
}

  
  

  
  
unsigned greatest_common_divisor (unsigned a, unsigned b)
{
  if (a > b)
  {
    return greatest_common_divisor ( a-b, b );
  }
  else if (b > a)
  {
    return greatest_common_divisor ( a, b-a );
  }
  else // (a == b)
  {
    return a;
  }
}

  
  

Motivation

A problem with some loops is that it is difficult to work out what each iteration is doing. Formal methods folks use the term "loop-invariant" to describe the condition that exists as the result of each iteration. An invariant can be added to code as either comments or assertions. The use of good identifier names can often reduce the need for this type of comment. But in the example above, there are no appropriate identifiers to name -- and do you really want to introduce a temp?
  
The solution is to replace the iteration with recursion. Unlike most procedural looping constructs, a recursive function call can be given a meaningful name -- this name should reflect the loop invariant. (In the example, the loop invariant is that the gcd of a and b is unchanged on each iteration). This can often lead to mode understandable code, and can also open new opportunities for other refactorings.
  
Many people worry about performace when using recursion. As always, performace isn't an issue until you profile the code and find the hotspots. However, even with this caveat, it is useful to dispel the myth:
  
On many c/c++ compilers (most, if you enable optimisation), the recursive version will compile to code that is more efficient! This is because the compiler can perform the tail-recursion optimisation to eliminate the function call overhead, leaving just the body of the loop as the limiting factor. In the first version, the exit criteria is checked at the start of each iteration; in the latter, it is only reached at the end of the calculation. So the iterative version keeps checking if (a != b), while the recursive version never makes that check. (it is, of course, possible to re-write the while-loop to cure this, but that optimisation doesn't have the readability/simplicity advantages of the recursive version)

Mechanics

• Identify the candidate loop. The loop should modify one or more scoped locals, and then return a result based on their final values.


• Move the loop into a new function.


• Compile and rerun tests.


• Replace the loop with a function that accepts the local variables, and which returns the final result.


• The implementation of the function should be an 'if' statement, which tests the looping condition (the condition expression in "while (condition) ...;"). The "then" clause should calculate/return the final result. The "else" clause should make the recursive call, with appropriately modified parameters.


• Compile and rerun tests.


• Now refactor the new function: You may be able to remove some temps, and find a better structure for the conditional in the recursive fuction

Example

Start with this code.
-
-  

unsigned foo (...)
{
  ...

  unsigned a = f();
  unsigned b = g();

  ...

  while (a != b)
  {
    if (a > b)
    {
      a -= b;
    }
    else if (b > a)
    {
      b -= a;
    }
  }

  unsigned c = a;

  ...
}

  
This code has a pretty horrible loop: fortunately, we know what it does:  
  

unsigned foo (...)
{
  ...

  unsigned a = f();
  unsigned b = g();

  ...

  unsigned c = greatest_common_divisor(a, b);

  ...
}

unsigned greatest_common_divisor (unsigned a, unsigned b)
{
  while (a != b)
  {
    if (a > b)
    {
      a -= b;
    }
    else if (b > a)
    {
      b -= a;
    }
  }
  return a;
}

  
  
I can now compile and test this.
  
Now that I've extracted the loop (which may, itself, be a sufficient refactoring), I can change it to a recursive form:
-
-  


unsigned greatest_common_divisor (unsigned a, unsigned b)
{
  if (a != b)
  {
    if (a > b)
    {
      a -= b;
    }
    else if (b > a)
    {
      b -= a;
    }
    return greatest_common_divisor(a, b);
  }
  else
  {
    return a;
  }
}

  
  
This form isn't much better yet. There are 2 further simplifications: we fir
st avoid modifying the parameters,
-
-


unsigned greatest_common_divisor (const unsigned a, const unsigned b)
{
  if (a != b)
  {
    if (a > b)
    {
      return greatest_common_divisor(a-b, b);
    }
    else if (b > a)
    {
      return greatest_common_divisor(a, b-a);
    }
   }
  else
  {
    return a;
  }
}
  
  
and, finally, we simplify the conditionals:  
  

unsigned greatest_common_divisor (const unsigned a, const unsigned b)
{
  if (a > b)
  {
    return greatest_common_divisor(a-b, b);
  }
  else if (b > a)
  {
    return greatest_common_divisor(a, b-a);
  }
  else // a == b
  {
    return a;
  }
}

***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Inheritance with Delegation

A subclass uses only part of a superclasses interface or does not want to inherit data.

Create a field for the superclass, adjust methods to delegate to the superclass, and remove the subclassing.



For more information see page 352 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Exception with Test

You are throwing an exception on a condition the caller could have checked first.

Change the caller to make the test first.

        double getValueForPeriod (int periodNumber) {
                try {
                        return _values[periodNumber];
                } catch (ArrayIndexOutOfBoundsException e) {
                        return 0;
                }
        }


        double getValueForPeriod (int periodNumber) {
                if (periodNumber >= _values.length) return 0;
                return _values[periodNumber];
        }


For more information see page 315 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)

Replace Error Code with Exception

A method returns a special code to indicate an error.

Throw an exception instead.

        int withdraw(int amount) {
                if (amount > _balance)
                        return -1;
                else {
                        _balance -= amount;
                        return 0;
                }
        }



        void withdraw(int amount) throws BalanceException {
                if (amount > _balance) throw new BalanceException();
                _balance -= amount;
        }


For more information see page 310 of Refactoring




***** 아름다운프로님에 의해서 게시물 복사 + 카테고리변경되었습니다 (2003-12-18 17:27)