int readByteFromFile(String fileName) throws IOException {
InputStream inputStream = new FileInputStream(fileName);
try {
return inputStream.read();
} finally {
try {
inputStream.close();
} catch (IOException ex) {
log("Hi Ade.");
}
}
}
String readStringFromFile(String fileName) throws IOException {
InputStream inputStream = new FileInputStream(fileName);
try {
DataInputStream dataInput = new DataInputStream(inputStream);
return dataInput.readUTF();
} finally {
try {
inputStream.close();
} catch (IOException ex) {
log("Hi Ade.");
}
}
}
You don't want to have to keep repeating this every time you want to do something similar, but slightly different. How do you refactor the similarities between these methods? The fact is that in Java I see lots of this sort of duplication - because in Java, getting rid of this sort of duplication is often verbose and clunky. However, it is possible, and sometimes very worthwhile.
interface DoWithFile {
Object execute(FileInputStream inputStream) throws IOException;
}
public Object withFile(String fileName, DoWithFile runThis) throws IOException {
FileInputStream inputStream = new FileInputStream(fileName);
try {
return runThis.execute(inputStream);
} finally {
try {
inputStream.close();
} catch (IOException ex) {
DuplicatedCode.log("Hi Ade.");
}
}
}
int readByteFromFile(String fileName) throws IOException {
return ((Integer) withFile(fileName,
new DoWithFile() {
public Object execute(FileInputStream inputStream)
throws IOException {
return inputStream.read();
}
})).intValue();
}
String readStringFromFile(String fileName) throws IOException {
return (String) withFile(fileName, new DoWithFile() {
public Object execute(FileInputStream inputStream)
throws IOException {
DataInputStream dataInput = new DataInputStream(inputStream);
return dataInput.readUTF();
}
});
}
Despite how horrible this looks - it is sometimes worth it.
def readByteFromFile(fileName):
inputStream = file(fileName)
try:
return inputStream.read(1)
finally:
try:
inputStream.close()
except:
log("Hi Ade.")
def readStringFromFile(fileName):
inputStream = file(fileName)
try:
dataInput = DataInputStream(inputStream)
return dataInput.readUTF()
finally:
try:
inputStream.close()
except:
log("Hi Ade.")
Passing a function as a parameter (as a closure) gives:
def withFile(fileName, function):
inputStream = file(fileName)
try:
return function(inputStream)
finally:
try:
inputStream.close()
except:
log("Hi Ade.")
def readByteFromFile(fileName):
def execute(inputStream):
return inputStream.read(1)
return withFile(fileName, execute)
def readStringFromFile(fileName):
def execute(inputStream):
dataInput = DataInputStream(inputStream)
return dataInput.readUTF()
return withFile(fileName, execute)
which hurts much less than the Java version (but slightly more than the Smalltalk version would - but that's another story). [Martin Fowler shows in his article how a Ruby caller of similar code would look "File.open(filename) {|f| doSomethingWithFile(f)}".]
Python doesn't have great syntax for defining closures - it would be nice to be able to define the two "execute" functions anonymously. You can do this for one of the methods using "lambda" but not the other because of the limitations of "lambda". In Smalltalk or Ruby the syntax for this is better. Here's the "lambda" version of the "readByteFromFile" function.
def readByteFromFile(fileName): return withFile(fileName, lambda inputStream: inputStream.read(1))
Following on from the previous article in the series, there are some "programming in the small" style things that I don't see as often as I'd expect.
Consider the following (Java) code:
int readValue() throws IOException {
InputStream file = new FileInputStream("temp.txt");
int result;
try {
result = file.read();
} finally {
file.close();
}
return result;
}
This can be written slightly shorter, and I think slightly better:
int readValue() throws IOException {
InputStream file = new FileInputStream("temp.txt");
try {
return file.read();
} finally {
file.close();
}
}
However, I hardly ever see other people using this style. This way of using finally came up when writing a previous article with Bjorn Freeman-Benson - he likes this style too. I don't expect to see this style all the time - just a lot more than I do, which is hardly ever.
Comments aren't working at the moment, but here's one by email from David Peterson (and he's right, you know. I've altered the code as David's suggestion - I originally had the code working but a bit wrong due to "brain fart"). (David's "I got rejected (blacklisted)" applies to everyone at the moment).
Hi Ivan,
It's better not to put the creation of the stream within the try.. finally. If the constructor throws an exception then you will end up calling close() on a null reference.
So, an even shorter (and safer) version is...
int readValue() throws IOException {
InputStream stream = new FileInputStream("temp.txt");
try {
return stream.read();
} finally {
stream.close();
}
}
David
P.S. I tried posting a comment, but I got rejected (blacklisted). :-(
Another comment, from Adewale Oshineye:
There's a reason people don't write methods like the one in your
example. The IOException that's thrown can be misleading. The call to
read() can throw an exception which then gets buried if the call to
close() also throws an exception.
The smallest safe version of that method looks something like:
int readValue() throws IOException {
InputStream inputStream = new FileInputStream("temp.txt");
try {
return inputStream.read();
} finally {
try {
inputStream.close();
} catch (IOException e) {
//we can't do anything about this except log it
LOG.warn("Problem closing inputStream", e);
}
}
}
My response: Thanks for the comment - I should have picked a better example - catching the possible IOException from the close is unrelated to the point of having a return inside a try finally and some code you want to execute after the return and my example has detracted from that point.