Therefore, take a part of the method that seems useful on its own. (To check this, see if you can find a good name for it.) Turn it into a separate method. Change the original method to use the new one.
Related patterns. If the new method would take a lot of parameters (originally these were local variables or method arguments), try MethodObject first.
Also described on page 110 of MartinFowler's RefactoringImprovingTheDesignOfExistingCode:
By the way, didn't we have a discussion somewhere about TooLongMethods?? Can we hope to reach a kind of agreement about the ideal method length in OO software? -- MarnixKlooster
See ItDepends
I wouldn't say that you do this when a method is too long, but when a method does more than one thing, or if it mixes high-level and low-level operations. I know that I've read a pattern by KentBeck (ComposedMethod in the SmalltalkBestPracticePatterns) that says not to mix levels within a method.
Therefore, the problem is that a method contains the wrong level of abstraction. The symptom is that the method "feels" too long. I have seen 40 line Smalltalk methods that weren't too long. I also have seen 5 line methods that were. -- DonRoberts
Can you give an example of a good 40 line SmallTalk methods. I have seen such things in C++, and sometimes even Java, because they are not parsimonious languages. But in SmallTalk? -- AamodSane
One example is a class initialization method that sets up some table:
initialize SomeClassVariable := Dictionary new. SomeClassVariable at: #someValue1 put: #someOtherValue1; ...many more values... at: #someValueN put: #someOtherValueN-- JohnBrant
Compare with TheSimplestCode. ExtractMethod seems to go directly against rule 4 (minimizes number of classes and methods). You might argue that it should only be done for facilitating 2 (no duplication) or 3 (expresses all the ideas you want to express), but that doesn't seem to cover all of its uses. ExtractMethod is clearly a useful and extremely common refactoring technique, but it seems to result in less "simple" code. How can you tell when it's appropriate? Is TheSimplestCode what you should write before you start refactoring, or what you want your code to look like all the time? (arguably this discussion would be better placed in RaiseAbstraction or FacadePattern or TheSimplestCode or...?) -- AnAspirant
"Expresses all the ideas you want to express" includes the ideas of reducing coupling and increasing cohesion. This are important ideas. (Someone wrote that ALL of the writing about good OO design can be summed up as reduce coupling and increase cohesion.)
Whenever I use ExtractMethod and MoveMethod, it is almost always reducing the number of responsibilities of a method or a class (increasing cohesion) and sometimes reducing coupling between two classes. -- KeithRay
Has anyone taken this to its extreme and always made every block a separate method? (E.g., each multi-statement loop body is extracted into a method, etc.) I can't imagine a reason for applying this refactoring so compulsively (other than perfecting one's talent for giving methods MeaningfulNames), but am curious whether anyone has tried it, or practices it regularly.
This is the type of refactoring that I mentioned in TestFirstAndFunctionalProgrammingSynergy. To me it's great for making the code more testable. -- DavidPlumpton
Yes, I do extract every block into its own method. It makes it much clearer as to what is going on and separates program control from program algorithms. When doing a loop, you can concentrate on getting the loop correct and visually verifying the loop. When doing the processing inside the loop, you can concentrate on get the processing correct and visually verifying the processing. --WayneMack
I've tried this, and of course found that I can't keep track of the helper methods. So I put them all in a MethodObject, which has been known to reveal a hidden abstraction. The effect of separating looping from processing is so powerful that I tend to give all inner loops their own methods. This can result in amazingly clean code
for(int outer=0;i<array.length;i++) doInnerLoops();-- DavidWright
This page mirrored in WikiPagesAboutRefactoring as of July 17, 2004