Please, don't abuse try/catch

You should have heard from experienced developers that you shouldn't abuse try/catch to handle a situation that is predictable and easily handled with an if statement. Even guys at [MS] preach this approach.

Take a look at this piece of code:

string fileName = "somefile.xml"; bool notFound; try { FileStream fs = File.Open(fileName, FileMode.Open); // do something } catch (FileNotFoundException) { notFound = true; }

While this method produces correct output (notFound=true) when file isn't found there are at least two problems in there:

  1. Method is slower when file isn't found (because of throwing exception overhead). This is just a small overhead.
  2. What's more annoying is that if you have Debugger/Exceptions... settings set to intercept all exceptions (shown in picture below) you will catch such exceptions even when there is nothing wrong with the code.

The reason why I am writting this post is that I get such an exception when creating an instance of IsolatedStorageFileStream class. Visual Studio correctly stopped on instance creation telling me that an exception of type FileNotFoundException occured.At first I was a bit puzzled why would I get FileNotFoundException when I was passing FileMode.Create flag. Doesn't make sense, right? Wrong. IsolatedStorageFileStream constructor internally implements similar (not same) code described above. It uses a handled exception for checking whether file exists even when using FileMode.Create!

Instead the code above could be rewritten as:

string fileName = "somefile.xml"; bool notFound; if (File.Exists(fileName) { FileStream fs = File.Open(fileName, FileMode.Open); // do something } else notFound = true;

This approach is more elegant, readable, faster and it doesn't throw anything on predictable problems. I guess [MS] guys could clean up a bit .net library.

UPDATE (gorazd pointed that there is a possibility that file disappears between File.Exists and File.Open - while I thought of this when I was writing this post I forgot to include it in the code). So, here is an improved v2:

string fileName = "somefile.xml"; bool notFound; if (File.Exists(fileName)) { try { FileStream fs = File.Open(fileName, FileMode.Open); // do something } catch (FileNotFoundException) { notFound = true; } } else notFound = true;

UPDATE: The good news is this behavior is a must only in Visual Studio 2003. Not because of a code change. No, there is actually an option to disable catching non-my-code exceptions. The option is turned on by default. So, here it is:

You'll find it in Tools/Options... Unfortunatelly that doesn't help if you are stuck in a Visual Studio 2003 world.

Comments (6) -

  • Laurent

    11/23/2006 6:34:05 PM | Reply

    I am heavily disturbed by dozens exceptions during ASP.NET 1.1 debugging sessions too.
    The same way as you when you use IsolatedStorageFileStream ): !

  • Miha Markic

    11/23/2006 6:50:29 PM | Reply

    That sucks, doesn't it? Perhaps there could be an option such as "disable breaking on not my code".

  • gorazd

    11/23/2006 6:59:37 PM | Reply

    well, there is a chance that file is deleted between Exists and actually opening it...

  • Miha Markic

    11/23/2006 8:05:36 PM | Reply

    Hi gorazd,

    Yep, that's the chance. In such case you should put both if and catch statements.

  • PetarR

    11/24/2006 4:35:37 AM | Reply

    "1. Method is slower when file isn't found (because of throwing exception overhead). This is just a small overhead."

    I looked at ctor of IsolatedStorageFileStream in .NET 1.1 with Refactor. It results that try/catch in question is executed when "mode" parameter is FileMode.Append (Opens the file if it exists and seeks to the end of the file, or creates a new file).

    Now, let us assume that this ctor with  FileMode.Append is called in majority of situations on existing files (seems logical because it is called Append and not Create).

    I think that means that their implementation performs faster because in majority of cases it requires 1 IO operation (file open) and your implementation 2 (file exists + file open).

    "Perhaps there could be an option such as disable breaking on not my code""

    It exists something like that, but in .NET 2.0.
    Look at DebuggerNonUserCodeAttribute Class.

  • Miha Markic

    11/24/2006 1:34:33 PM | Reply

    Hi Petar,

    1. The speed difference isn't that important IMO.
    2. I don't think DebuggerNonUserCodeAttribute can help in this scenario. And even if it helped you would have problems turning it on or off. Anyway, I updated my article.

Loading