Geeks With Blogs
Eron Wright - All Killer No Filler blog

I am so sick of seeing this incorrect usage of try..finally:

Stream s = null;
try { 
    s = new FileStream(...);
    ...
}
finally {
    if(s!=null) s.Close();
}

This is WRONG. The work done in the finally block need only be done if the stream is opened.  Until the assignment of 's' completes, the finally block is not to be executed.  The 'if' statement is basically checking that fact.  The correct usage is:

Stream s = new FileStream(...);
try {     
    ...
}
finally {
    s.Close();
}

No excuses!  Programmers are lucky that the 'if' check can even be done - the anti-pattern doesn't work in general.  Consider this scenario involving thread synchronization:

Monitor.Enter();
try {     
    ...
}
finally {
    Monitor.Exit();
}

To write the above using the anti-pattern, one would need to be able to check that the monitor was entered, which may or may not be possible. Without a check, the anti-pattern might exit a monitor that was not entered.

With respect to exception handling, to catch an exception for Enter through to Exit the correct pattern is:

try {
    Monitor.Enter();
    try {     
        ...
    }
    finally {
        Monitor.Exit();
    }
} catch(Exception ex) { ... }

To handle an exception on Enter separately from the rest, use:

try {
    Monitor.Enter();
}
catch(Exception ex) { ... }
try {     
    ...
}
catch(Exception ex) { ... }
finally {
    Monitor.Exit();
}

Microsoft programmers use this anti-pattern all the time.  Stop it!

Posted on Saturday, January 3, 2004 11:32 AM Technology | Back to top


Comments on this post: Anti-pattern: incorrect try..finally usage

# re: Anti-pattern: incorrect try..finally usage
Requesting Gravatar...
Your corrected version doesn't allow for exceptions being thrown by the new FileStream(...) statement.
Left by Iain on Jan 03, 2004 4:50 PM

# re: Anti-pattern: incorrect try..finally usage
Requesting Gravatar...
Iain, I have updated the post in light of your concern. Obviously other variations are possible.
Left by Eron Wright on Jan 05, 2004 1:07 PM

# re: Anti-pattern: incorrect try..finally usage
Requesting Gravatar...
You could just simply avoid the entire first issue and use the 'using' keyword: It automatically disposes of the created object if an exception is thrown.
Left by Omer van Kloeten on May 06, 2004 11:41 AM

# re: Anti-pattern: incorrect try..finally usage
Requesting Gravatar...
Your pattern 'to catch an exception for Enter through to Exit' serves to move the exception handling code even further from the source of the exception. Code present in the outer catch which serves to handle exceptions from the Monitor.Enter() (or even new FileStream(...)) would not intuitively be linked to the code that caused the exception, since it's so far away.

In my view, the original so-called anti-pattern is easier to read!
Left by Joe on Dec 13, 2005 8:31 AM

# re: Anti-pattern: incorrect try..finally usage
Requesting Gravatar...
Bullshit.
First code is completely different than the second. In first You catch exceptions from FileStream constructor. In second You don't.
Left by Jiggaboo on Oct 28, 2009 11:29 AM

Your comment:
 (will show your gravatar)


Copyright © Eron Wright | Powered by: GeeksWithBlogs.net