is there a reason Eclipse gives me the following resource leak warning: Resource leak: 'br' is never closed" ? The code I am talking about is at the bottom of this post.
I thought my finally block had it all covered, my reasoning:
So what am I missing? Or could this be an eclipse bug?
Kind regards!
S.
public static String fileToString(String fileName, String encoding) throws IOException {
InputStream is;
InputStreamReader isr;
BufferedReader br;
Closeable res = null;
try {
is = new FileInputStream(fileName);
res = is;
isr = new InputStreamReader(is, encoding);
res = isr;
br = new BufferedReader(isr);
res = br;
StringBuilder builder = new StringBuilder();
String line = null;
while ((line = br.readLine()) != null) {
builder.append(line);
builder.append(LS);
}
return builder.toString();
} finally {
if (res != null) {
res.close();
}
}
}
Eclipse probably just isn't understanding the shuffling you're doing with the res
variable.
I recommend using the try-with-resources statement (available in Java 7 and up, so three and a half years now), it dramatically simplifies these sorts of chains:
public static String fileToString(String fileName, String encoding) throws IOException {
try (
InputStream is = new FileInputStream(fileName);
InputStreamReader isr = new InputStreamReader(is, encoding);
BufferedReader br = new BufferedReader(isr)
) {
StringBuilder builder = new StringBuilder();
String line = null;
while ((line = br.readLine()) != null) {
builder.append(line);
builder.append(LS);
}
return builder.toString();
}
}
If you can't use try-with-resources, you probably want something like the Apache Commons IOUtils
class's closeQuietly
methods (either literally that one, or your own) rather than shuffling res
around, which is awkward to read and I daresay prone to maintenance issues.
Using IOUtils
might look like this:
public static String fileToString(String fileName, String encoding) throws IOException {
InputStream is = null;
InputStreamReader isr = null;
BufferedReader br = null;
try {
is = new FileInputStream(fileName);
isr = new InputStreamReader(is, encoding);
br = new BufferedReader(isr)
StringBuilder builder = new StringBuilder();
String line = null;
while ((line = br.readLine()) != null) {
builder.append(line);
builder.append(LS);
}
br.close();
return builder.toString();
}
finally {
IOUtils.closeQuietly(br, isr, is);
}
}
Note how I use a normal close
in the try
, but then ensure cleanup in the finally
.
But try-with-resources is the better answer, as it's more concise and hooks into the new(ish) "suppressed exceptions" stuff.
Side note: There's no reason for the = null
initialization of line
, you assign it on the next line.
Side note 2: If the file is likely to be of any size, consider finding out how big it is in advance and setting the capacity of the StringBuilder
in the constructor. StringBuilder
's default capacity is 16, so a file of even a few hundred bytes involves several reallocations of StringBuilder
's internal buffer.