Minimize nested try/catch blocks where possible in the same function.
In the following example, the whole loop could have been written with just one try block - the others were not necessary. A second finally block added more complexity.
private boolean compareFile(File file1, File file2)
{
boolean same = true;
if (!file1.exists() || !file2.exists() || !file1.isFile()
|| !file2.isFile() || file1.length() != file2.length())
{
return false;
}
BufferedInputStream f1 = null;
BufferedInputStream f2 = null;
try
{
f1 = new BufferedInputStream(new FileInputStream(file1));
// *see below
try
{
f2 = new BufferedInputStream(new FileInputStream(file2));
try
{
int b = 0;
while ((b = f1.read()) >= 0)
{
if (b != f2.read())
{
same = false;
break;
}
}
}
finally
{
// clean up streams when finished
f2.close();
}
}
finally
{
f1.close();
}
}
catch (Exception e)
{
logger.error(e);
}
return same;
}
*If more code was added here that threw an exception, f1 might never be closed.
private boolean compareFile(File file1, File file2)
{
boolean same = true;
if (!file1.exists() || !file2.exists() || !file1.isFile()
|| !file2.isFile() || file1.length() != file2.length())
{
return false;
}
BufferedInputStream f1 = null;
BufferedInputStream f2 = null;
try
{
f1 = new BufferedInputStream(new FileInputStream(file1));
f2 = new BufferedInputStream(new FileInputStream(file2));
int b = 0;
while ((b = f1.read()) >= 0)
{
if (b != f2.read())
{
same = false;
break;
}
}
}
catch (Exception e)
{
log.error(e);
}
finally
{
// clean up streams when finished
if (f1 != null)
try {f1.close();} catch(Exception e) {log.error(e);};
if (f2 != null)
try {f2.close();} catch(Exception e) {log.error(e);};
}
return same;
}
This improved code catches exceptions just at one level, so the code is neatly contained and doesn't nest too deeply.
Text and examples are Copyright © 2015 Murray Brandon.
