Tuesday, February 17, 2009

A pretty nice DRY Template Method Refactoring.

I had a class with already duplicated code as follows (Excluding irrelevant parts):



public class FileWriterImpl implements FileWriter {

public void write(A1 a1) {
BufferedWriter writer = null;
try {
File file = new File(filePathFinal);
file.delete();
if (!file.exists()) {
file.createNewFile();
}
writer = new BufferedWriter(new java.io.FileWriter(new File(filePathFinal)));
write(a1, writer);
} catch (IOException e) {
throw new A1GenerationException(ErrorCodeConstants.FILE_CREATION_ERROR,new Object[]{e.getMessage()});
} finally {
try {
writer.flush();
writer.close();
} catch (Exception e) {

}
}
}

public void writeOneLineToFileAndCloseWriter(Line line) {
if (line == null) {
logger.info("LA LINEA ES NULA. NO SE ESCRIBE NADA EN EL FICHERO");
return;
}
BufferedWriter writer = null;
try {
File file = new File(filePathFinal);
if (!file.exists()) {
file.createNewFile();
}
writer = new BufferedWriter(new java.io.FileWriter(new File(filePathFinal), true));
writeLineToWriter(line, writer);
} catch (IOException e) {
throw new A1GenerationException(ErrorCodeConstants.FILE_CREATION_ERROR,new Object[]{e.getMessage()+filePathFinal});
} finally {
try {
writer.flush();
writer.close();
} catch (Exception e) {

}
}
}

}

As you can see there is very common code beetwen the two methods. But i didn't do anything about it. Until yet another method needed to be introduced!!..
So i decided to refactor it to eliminate duplicated code.

I observed that the file accessing stuff was basically a template method with the only differences in the code being the parts that come after the "writer = new BufferedWriter(new java.io.FileWriter(new File(filePathFinal)));".

Not implementing the exact Template Method Pattern (As is in literature) but rather the way Spring implements it in JdbcTemplate and alikes, i created a CallbackInterface:

interface FileWritingOperation{
void execute(BufferedWriter writer) throws IOException;
}

Objects implementing this interface will receive a writer and execute any logic they want on it. But zero Boilerplate code. Just the functionality they need.

Then i refactored the previous method into the following more generic method:

public void doInFileWriterResourceSecure(FileWritingOperation operation) {
BufferedWriter writer = null;
try {
File file = new File(filePathFinal);
if (!file.exists()) {
file.createNewFile();
}
writer = new BufferedWriter(new java.io.FileWriter(new File(filePathFinal), true));
operation.execute(writer);
} catch (IOException e) {
throw new A1GenerationException(ErrorCodeConstants.FILE_CREATION_ERROR, new Object[] { e.getMessage() + filePathFinal });
} finally {
try {
writer.flush();
writer.close();
} catch (Exception e) {

}
}
}

As you can see the method now receives a FileWritingOperation as a parameter. The method is in charge of managing the File resources, and wher appropiate invokes the execute method of the FileWritingOperation passed.

Now all three individual methods look like the following:

public void write(final A1 a1) {
doInFileWriterResourceSecure(new FileWritingOperation(){
public void execute(BufferedWriter writer) throws IOException {
write(a1, writer);
}

});
}

public void writeBlock(final Block lineBlock) {
if (lineBlock == null) {
logger.info("Bloque Nulo. NO SE ESCRIBE NADA EN EL FICHERO");
return;
}

doInFileWriterResourceSecure(new FileWritingOperation(){
public void execute(BufferedWriter writer) throws IOException {
for (Line line : lineBlock.getLines()) {
writeLineToWriter(line, writer);
}
}

});
}


public void writeOneLineToFileAndCloseWriter(final Line line) {
if (line == null) {
logger.info("LA LINEA ES NULA. NO SE ESCRIBE NADA EN EL FICHERO");
return;
}
doInFileWriterResourceSecure(new FileWritingOperation(){
public void execute(BufferedWriter writer) throws IOException{
writeLineToWriter(line, writer);
}
});
}


And there is no more Repetition and definetly a more elegant code.