Don't Code Like This!

I was trying an open source web framework yesterday, and couldn't get it to work so I stepped through some of the code on the server with a debugger, and came across some code fragments that I thought I would submit to the Hall of Shame.

Besides, it's fun to blog about this, because each time I mention coding style I invariably get passionate responses from people defending their beloved style.

In the following, the class and method names have been changed to protect the guilty.

At first, I simply got a generic JspException. The code was doing this:

        ...
    } catch (ConfigNotFoundException ex) {
        throw new JspException(msg);
    }
The original stack trace is not chained to the new exception! Please pass it in - JspException has a constructor which takes both a message and the root cause exception, ex in the above. If you're using some Exception class which is older and doesn't let you pass in the root cause, such as DOMException, then first create the exception, then call the initCause method on it, and finally throw the exception object.

The second code fragment I came across was this:

    try {
        return findConfig(servletContext).getResult(foo, servletContext);
    } catch (NullPointerException ex) {
        throw new ConfigNotFoundException("Can't find configuration");
    }
The above code is using exceptions for flow control. Rather than first calling findConfig, then checking if it is null, it just goes ahead and tries calling it anyway, relying on the NullPointerException to handle this case.

The code also has the bad effect of catching all unintended null pointer exceptions in the subsequent getResult method call. These are unrelated to the ability to find the configuration, so the error message would be misleading. (And again the original exception is not chained).

Comments:

re style ... is it just me (being old school) or has including multiple return statements w/in a method become quite vogue? to me, chasing down scattered returns just feels ... well ... dirty. that and i think always having one include at the bottom of a method results in more normalized code, or so goes my hunch.

Posted by gonzo on July 13, 2005 at 01:07 PM PDT #

Well I disagree with you regarding return statements - I've already written about those: http://blogs.sun.com/roller/page/tor?entry=onlyonereturn_considered_harmful

Posted by Tor Norbye on July 13, 2005 at 01:30 PM PDT #

ahhh, thx. i'm still waiting to be "enlightened" on this one :)

is see there are 9 comments on that blog'o interest so i'll pop a cap from my favorite bottle'o beer and take it all in.

Posted by gonzo on July 13, 2005 at 02:10 PM PDT #

k ... my take away post reading the "OnlyOneReturn Considered Harmful" thread ... balance is the key. guess i'm in a habit of returning only once (old habits die hard) but i can see for trivial modules where it can help. not sure i'll be retrofitting any code though to this new fangled coding style all the kidz r tawlk'n 'bout.

rock on!

Posted by gonzo on July 13, 2005 at 02:19 PM PDT #

Post a Comment:
Comments are closed for this entry.
About

Tor Norbye

Search

Archives
« May 2015
SunMonTueWedThuFriSat
     
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
      
Today