Extract Method

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. If the code has several validations, see AbortiveValidations

Also described on page 110 of MartinFowler's RefactoringImprovingTheDesignOfExistingCode:

"You have a code fragment that can be grouped together."

"Turn the fragment into a method whose name explains the purpose of the method."

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

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. These 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();

   state N:
      performAppropriatelyNamedAction();
      change to state M;
      break;

Maybe there's an analogy to the various types of pasta code. This is clearly not SpaghettiCode, or LasagnaCode, or RavioliCode. Maybe it is SubmarineSandwichCode? - goes on for miles with lots of stuff inside.

Consider it time to ForgetTheDebugger perhaps?

Basically many programmers who consider themselves experts have no respect for: debugger jumping, YAGNI abstractions, context switching, or horizontal scrolling (this.that.method.submethod.scrolltillyoudie.etc...)

IMO those aren't forces that deserve respect. In any reasonably large code base, the difference between using 30 line methods and 5 line methods won't significantly change the call stack depth. Debuggers are there to jump for us. YAGNI applies to features, not methods. Horizontal scrolling is decreased by ExtractMethod, not increased, because each new method shifts code back to the left side of the screen. Closing debugger windows isn't a nightmare in any debugger I've ever used. Most of them will close them all automatically for you, won't they? -- EricHodges

This page mirrored in WikiPagesAboutRefactoring as of April 29, 2006