Long Method
Symptoms
-
Large number of lines. (I'm immediately suspicious of any method with more than 5 to 10 lines.)
Causes
I think of it as the Columbo syndrome. Columbo was the detective who always had “just one more thing.” A method starts down a path and, rather than break the flow or identify the helper classes, the author adds one more thing. Code is often easier to write than it is to read, so there's a temptation to write blocks that are too big.
What to Do
-
Use Extract Method to break up the method into smaller pieces. Look for comments or white space delineating interesting blocks. You want to extract methods that are semantically meaningful, not just introduce a function call every seven lines.
-
You may find other refactorings (those that clean up straight-line code, conditionals, and variable usage) helpful before you even begin splitting up the method.
Payoff
Improves communication. May expose duplication. Often helps new classes and abstractions emerge.
Discussion
-
People are sometimes worried about the performance hit from increasing the number of method calls, but most of the time this is a nonissue. By getting the code as clean as possible before worrying about performance, you have the opportunity to gain big insights that can restructure systems and algorithms in a way that dramatically increases performance.
-
Don't game the metrics; the goal of using Extract Method is to use it in a way that increases insight.
Contraindications
It may be that a somewhat longer method is just the best way to express something. (Like almost all smells, the length is a warning sign—not a guarantee—of a problem.)
EXERCISE 4
Long Method
Consider this code. (Online at www.xp123.com/rwb)
Machine.java
public class Machine { String name; String location; String bin; public Machine(String name, String location) { this.name = name; this.location = location; } public String take() { String result = bin; bin = null; return result; } public String bin() { return bin; } public void put(String bin) { this.bin = bin; } public String name() {return name;} }
Robot.java
public class Robot { Machine location; String bin; public Robot() {} public Machine location() {return location;} public void moveTo(Machine location) {this.location = location;} public void pick() {this.bin = location.take();} public String bin() {return bin;} public void release() { location.put(bin); bin = null; } }
RobotTest.java
import junit.framework.*; public class RobotTest extends TestCase{ public RobotTest(String name) {super(name);} public void testRobot() { Machine sorter = new Machine("Sorter", "left"); sorter.put("chips"); Machine oven = new Machine("Oven", "middle"); Robot robot = new Robot(); assertEquals("chips", sorter.bin()); assertNull(oven.bin()); assertNull(robot.location()); assertNull(robot.bin()); robot.moveTo(sorter); robot.pick(); robot.moveTo(oven); robot.release(); assertNull(robot.bin()); assertEquals(oven, robot.location()); assertNull(sorter.bin()); assertEuals("chips", oven.bin()); } }
Report.java
import java.util.*; import java.io.*; public class Report { public static void report(Writer out, List machines, Robot robot) throws IOException { out.write("FACTORY REPORT\n"); Iterator line = machines.iterator(); while (line.hasNext()) { Machine machine = (Machine) line.next(); out.write("Machine " + machine.name()); if (machine.bin() != null) out.write(" bin=" + machine.bin()); out.write("\n"); } out.write("\n"); out.write("Robot"); if (robot.location() != null) out.write(" location=" + robot.location().name()); if (robot.bin() != null) out.write(" bin=" + robot.bin()); out.write("\n"); out.write("========\n"); } }
ReportTest.java
import junit.framework.TestCase; import java.util.ArrayList; import java.io.PrintStream; import java.io.StringWriter; import java.io.IOException; public class ReportTest extends TestCase { public ReportTest(String name) {super(name);} public void testReport() throws IOException { ArrayList line = new ArrayList(); line.add(new Machine("mixer", "left")); Machine extruder = new Machine("extruder", "center"); extruder.put("paste"); line.add(extruder); Machine oven = new Machine("oven", "right"); oven.put("chips"); line.add(oven); Robot robot = new Robot(); robot.moveTo(extruder); robot.pick(); StringWriter out = new StringWriter(); Report.report(out, line, robot); String expected = "FACTORY REPORT\n" + "Machine mixer\nMachine extruder\n" + "Machine oven bin=chips\n\n" + "Robot location=extruder bin=paste\n" + "========\n"; assertEquals(expected, out.toString()); } }
-
In Report.java, circle four blocks of code to show which functions you might extract in the process of refactoring this code.
-
Rewrite the report() method as four statements, as if you had done Extract Method for each block.
-
Does it make sense to extract a one-line method?
-
Long methods are trivially easy to spot, yet they seem to occur often in real code. Why?