In most cases I start refactoring when I recognize the code has begun to smell badly (as a collegue of mine uses to call it). In the past week I frequently refactored duplicated code. Eclipse provides you with excellent tools to do so, but sometimes the code needs to be prepared to enable Eclipse to perform its magic. The pattern recognition of Eclipse is far less flexible than any human beings pattern recognition. Frequently Eclipse misses repetitions that are completely obvious to you. There are some tricks to help Eclipse.
By the way, this article is about concepts. It is not about using a particular tool, nor is it about a particular language. This article provides you with screen shots of Eclipse and Java, but of course the ideas are applicable to any procedural language and to any IDE capable of refactoring. I suppose this includes Netbeans and IntelliJ's IdeaJ, just to name a few tools in the java world.
Consider the following code:
public abstract class Duplications { public abstract void saveFile(String filename, String fileContent) throws IOException; public abstract String readFile(String filename) throws IOException; public abstract void log(String level, String message); public void createFile() { try { saveFile("Greetings.txt", "Beyond Java says \"Hello World!\""); } catch (IOException error) { // please provide better error diagnosis in real life! log("Error", "Can't write the file due to an exception"); } } public void copyFile() { try { String fileContents = readFile("OldFileName.txt"); try { saveFile("Greetings.txt", fileContents); } catch (IOException error) { // please provide better error diagnosis in real life! log("Error", "Can't write the file due to an exception"); } } catch (IOException error) { // please provide better error diagnosis in real life! log("Error", "Can't read the file"); } } }You can clearly see that this code has been copied two times:
- The original code is the methode createFile().
- Writing the methode copyFile(), I copied the createFile() implementation and replaced the method name "saveFile" by "readFile". Of course, I had to adapt the log message, too.
- The second copy resides inside the first copy. It saves something to the same file, so I merely had to correct the second parameter of the saveFile() methods.
I encounter this kind of code duplication very often. It's obvious that the code has been copied, so it should be easy to move these duplication into a single method. Unfortunately, each copy has been modified slightly, so the factorization is not that easy.
In most cases, I rely heavily on Eclipses capabilities to recognize code duplications. Eclipse recognizes literal duplications only, so we have to do some preparations the help eclipse. Our first goal is to remove the second duplication. This should be easy, because the code looks almost the same. The only difference is the file content. To make the code look identical, select the text "Beyond Java says \"Hello World!\"" and type ALT+SHIFT+L to open the dialog to convert this constant string into a variable:
Make sure the variable has the same name as in the second copy (i.e. fileContents). The next step is to select the code and activate "extract methods" by typing ALT+SHIT-M. You should see this dialog:
This is bad luck - eclipse did not discover the duplication! So it is no use to click "ok". Instead, you have to move the newly created variable before the try statement. Repeat "extract method" again, and you will be rewarded by the dialog:
Now there remains a duplication, that is not easy to eliminate in java:
- The try-catch-block is duplicated, but the log message is different. You can fix this by moving the log message into a parameter passed to the method to be created.
- The algorithms inside the try-blocks are different. You can fix this by putting the algorithm into a class or a closure.
I guess hardly nobody will seriously think about introducing classes in trivial cases like my example, so we can't improve our code any further. However, if your language knows closure with a simple syntax - e.g. Groovy, Scala or maybe Java 8 - it is worth the pain to extract the algorithm into a variable. Having done so, you can extract the remaining code into a method, yielding a source code like this:
public abstract class Duplications { public abstract void saveFile(String filename, String fileContent) throws IOException; public abstract String readFile(String filename) throws IOException; public abstract void log(String level, String message); private void saveGreetings(String fileContents) { Closure algorithm = { saveFile("Greetings.txt", fileContents) } def errorMessage = "Can't write the file due to an exception" executeWithNiceLogging(algorithm, errorMessage) } public void createFile() { saveGreetings("Beyond Java says \"Hello World!\""); } private executeWithNiceLogging(Closure algorithm, String errorMessage) { try { algorithm.call(); } catch (IOException error) { // please provide better error diagnosis in real life! log("Error", errorMessage); } } public void copyFile() { Closure algorithm = { saveGreetings(readFile("OldFileName.txt")) } def errorMessage = "Can't read the file" executeWithNiceLogging(algorithm, errorMessage) } }I made this code more compact by inlining the variable "fileContents". This is a common technique in the art of refactoring: Introduce a variable in order to make two code repetions make exactly identical, convert the code duplication into a method and the new variable into a parameter of the method and get rid of it again by inlining it. Maybe you want to get rid of the variables "errorMessage" and "algorithm" in the Groovy example, reducing the class length to 23 lines. The original implementation had 28 lines. In real life, examples tend to be more complicated, resulting in much more impressive class length reductions. However, if you take the simplicity of the task into account, even such a small reducation is quite impressive. And - at least im my eyes - the code has become more readable after moving the boiler plate code into utility methods:
public abstract class Duplications { public abstract void saveFile(String filename, String fileContent) throws IOException; public abstract String readFile(String filename) throws IOException; public abstract void log(String level, String message); // boiler plate code private executeWithNiceLogging(Closure algorithm, String errorMessage) { try { algorithm.call(); } catch (IOException error) { // please provide better error diagnosis in real life! log("Error", errorMessage); } } // code interesting to business public void createFile() { saveGreetings("Beyond Java says \"Hello World!\""); } // code interesting to business private void saveGreetings(String fileContents) { executeWithNiceLogging( { saveFile("Greetings.txt", fileContents) }, "Can't write the file due to an exception") } // code interesting to business public void copyFile() { executeWithNiceLogging ({ saveGreetings(readFile("OldFileName.txt")) }, "Can't read the file") } }