Exceptions carefully wrapped using java.lang.Exception

Xenoveritas

It's fairly common to find Java code that "throws Exception" instead of actually, say, handling exceptions. Compiler errors are annoying, and it's just easier to "throws Exception" and ignore the problem than create the large list of silly exceptions that may be thrown in the method.

However, I wasn't expecting to see this in a file recently added to version control:

public Object readFile() throws Exception {
 BufferedReader r = openFile();
 Object data = parseFile(r);
 try {
 r.close();
 } catch (IOException e) {
 throw new Exception("Unable to close reader", e);
 }
 return data;
}

/**
* Opens a file with a BufferedReader.
*/
private BufferedReader openFile() throws Exception {
 FileReader r = null;
 try {
 r = new FileReader(fileName);
 } catch (FileNotFoundException e) {
 throw new Exception("File not found: " + fileName, e);
 }
 return new BufferedReader(r);
}

Now, ignoring everything else that's wrong with that (especially since some of it has been made more generic to mask the code), look at the exception handling. Instead of "throws IOException" like I'd expect, the code's wrapping the specific exception in the root-level exception object. As far as I can tell, this is an attempt to make the error messages more "user-friendly" - except:

The strings really are hardcoded, with no attempt to allow localization.
The error messages are now be generated within the implementation code and not the user-interface, mixing model and view.

What's even worse is that the correct code is actually easier to read:

public Object readFile() throws IOException {
 BufferedReader r =
 new BufferedReader(
 new FileReader(fileName));
 try {
 return parseFile(r);
 } finally {
 r.close();
 }
}

And all without wrapping exceptions in more generic types.

VGR

Just more evidence that most of the people writing Java don't know how
to use exceptions. I suppose it's the inevitable result of making
Java's syntax so similar to C++, thus leading C/C++ people to believe
they can hit the ground running with their current skill set.

If your environment doesn't have regular code reviews, then you
probably ought to let the author know directly that his method is very
very error prone, since it's forcing callers to catch things like
NullPointerExceptions.

maratcolumn1

That's nothing. I err... know some people who use exceptions in the following
way (unfortunately it cannot be written in Java):

public int readFile(out data) {
 try {
 BufferedReader r = openFile();
 data = parseFile(r);
 r.close();
 } catch (IOException e) {
 return Constants.EFILENOTFOUND;
 } catch (OutOfMemoryException e) {

 return Constants.ENOMEMORY;

 }
 return Constants.SUCCESS;
}

VGR

I may be able to beat that. I worked with a guy who wrote a Java
class named SharedMemory. I wish I were making that up.

kiriran

at least they are handling errors in some way. I had to fix a bug the other day and found this piece of code

try {
// code that throws an IOException
} catch (Exception e) {
// pssst ... ^_^
}

no big deal but the comment amuses me

w280sax

My favorite Java error handling of late...except I saw this kind of code all over the place.

public SubClass cast(SuperClass obj) {
if (obj instanceof SubClass) {
return (SubClass) obj;
}
else
{
throw new ClassCastException("Can't cast " + obj.getClass() + " to SubClass");

}

}

makomk

@kiriran said:

at least they are handling errors in some way. I had to fix a bug the other day and found this piece of code

try {
// code that throws an IOException
} catch (Exception e) {
// pssst ... ^_^
}

no big deal but the comment amuses me

// WTF? o_O