Blog Stats
  • Posts - 24
  • Articles - 0
  • Comments - 61
  • Trackbacks - 92

 

Saturday, January 03, 2004

Anti-pattern: incorrect try..finally usage

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!

 

 

Copyright © Eron Wright