Exceptions carefully wrapped using java.lang.Exception



  • 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:

    <font face="Courier New">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);
    }</font>

    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:

    1. The strings really are hardcoded, with no attempt to allow localization.
    2. 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:

    <font face="Courier New">public Object readFile() throws IOException {
        BufferedReader r =
                new BufferedReader(
                        new FileReader(fileName));
        try {
    </font><font face="Courier New">        return parseFile(r);
        } finally {
            r.close();
        }
    </font><font face="Courier New">}
    </font>
    And all without wrapping exceptions in more generic types.



  • 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.






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



    <font face="Courier New">public int readFile(out data) {
    </font><font face="Courier New">    try {
    </font><font face="Courier New">        BufferedReader r = openFile();
            data = parseFile(r);
            r.close();
        } catch (IOException e) {
            return Constants.EFILENOTFOUND;
        } catch (OutOfMemoryException e) {

            return Constants.ENOMEMORY;

        }
        return Constants.SUCCESS;
    }
    </font>




  • 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.



  • 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



  • 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");

        }

    }



  • @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


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.