228

Which one:

using (var myObject = new MyClass())
{
   try
   {
      // something here...
   }
   catch(Exception ex)
   {
      // Handle exception
   }
}

OR

try
{
   using (var myObject = new MyClass())
   {
      // something here...
   }
}
catch(Exception ex)
{
   // Handle exception
}
Xaqron
  • 29,931
  • 42
  • 140
  • 205
  • 9
    Just a note: one should be careful to only catch exceptions that can actually be _handled_ (corrected), except for logging, or wrapping them. – John Saunders Jan 04 '11 at 16:56
  • 1
    Please keep in mind that also the last `}` of the `using` statement can throw an exception [as reminded here](https://msdn.microsoft.com/en-us/library/aa355056%28v=vs.110%29.aspx). – Giulio Caccin Dec 21 '15 at 14:13
  • 1
    TIL that the debugger (in VS) will not call the dispose method if you use the first block of code. Because the using statement itself can throw an exception, it help me to use the second block to ensure the implied `finally` called the dispose method. – ShooShoSha Jan 28 '16 at 18:39

9 Answers9

119

I prefer the second one. May as well trap errors relating to the creation of the object as well.

Jonathan Wood
  • 65,341
  • 71
  • 269
  • 466
  • 13
    I disagree with this advice. If you are expecting the object creation to throw an error, then any handling of that exception *must* go outside. If there is some question about where the handling should go, then the exception that is expected must be something else—unless you are advocating catching any random exception that may or may not be anticipated, which is a classic anti-pattern (outside of a process or thread's Unhandled Exception Handler). – Jeffrey L Whitledge May 26 '11 at 21:42
  • 3
    @Jeffrey: The approach I described has served me well and I've been doing this a long time. No one said anything about *expecting* object creation to fail. But by wrapping an operation that could *potentially* fail in a `try` block, which allows you to pop an error message if something fails, the program now has the ability to recover and inform the user. – Jonathan Wood May 26 '11 at 22:12
  • Your answer is correct but continues the suggestion that the try/catch _has_ to be there (immediately) at all times. – H H May 26 '11 at 22:22
  • 20
    I think first one has merit as well, consider a DB transaction `using( DBConnection conn = DBFactory.getConnection())` which would need to be rolled back in case of an exception that occurred. Seems to me that both have their place. – wfoster Jun 03 '11 at 21:32
  • @wfoster In that case you'd probably better always rollback the transaction when `using` exits, except in case of an explicit commit. – Teejay Dec 04 '15 at 18:33
  • 1
    That will also trap errors related to the **disposal** of the object. – Ahmad Ibrahim Feb 06 '18 at 14:55
  • 1
    @JasonC: That new syntax is nothing more than syntactic sugar that simply uses the current code block to determine scope. It does not make this question moot. You can still control that scope. – Jonathan Wood Jan 17 '22 at 16:26
  • @JonathanWood Yes; you are correct and I knew this. I'm really not sure why I wrote "moot" , I'm going to blame that one on lack of coffee. Sorry about that; thanks for calling it out. – Jason C Jan 17 '22 at 16:45
  • Let's try this again: Also, C# 8 added a neat new syntax that makes both forms a bit more concise ([docs](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/using-statement) and I've got an example below). The main difference is, if the OP's example has code after those `using` blocks, the new syntax doesn't do much if you're looking for an *exact* match to the scope of the examples. But I've found that in most cases, it helps. – Jason C Jan 17 '22 at 16:50
  • @JasonC: My point was that, using the new syntax, you can still choose whether the `using` statement goes before the `try` block or within it. That would duplicate the two variations he originally asked about. – Jonathan Wood Jan 17 '22 at 17:24
50

Since a using block is just a syntax simplification of a try/finally (MSDN), personally I'd go with the following, though I doubt it's significantly different than your second option:

MyClass myObject = null;

try
{
    myObject = new MyClass();
    //important stuff
}
catch (Exception ex)
{
    //handle exception
}
finally
{
    if (myObject is IDisposable)
    {
        myObject.Dispose();
    }
}
chezy525
  • 4,025
  • 6
  • 28
  • 41
  • 5
    Why do you think adding a `finally` block is preferable to the `using` statement? – Cody Gray - on strike Jan 04 '11 at 04:25
  • 12
    Adding a `finally` block that disposes an IDisposable object is what a `using` statement does. Personally, I like this instead of the embedded `using` block because I think it more cleanly states where everything is happening, and that it's all on the same "level". I also like this more than several embedded `using` blocks... but it's all just my preference. – chezy525 Jan 04 '11 at 15:47
  • 11
    If you implement much exception handling, you must really enjoy typing! That "using" keyword has been around for a while and it's meaning is quite clear to me. And using it helps make the rest of my code clearer by keeping the amount of clutter to a minimum. – Jonathan Wood Jan 06 '11 at 18:03
  • 3
    This is incorrect. The object must be instantiated outside the `try` statement for it to be disposed within the `finally` statement; otherwise, it will throw a compiler error: "Use of unassigned local variable 'myObject'" – Steve Konves Jul 02 '12 at 15:38
  • 1
    @SteveKonves, I suppose you're technically correct, the first line should be `var myObject=null;` ...Though I wouldn't consider that a downfall of any pseudo-code... – chezy525 Jul 06 '12 at 16:37
  • 3
    Technically, that won't compile either. `Cannot assign null to implicitly-typed local variable` ;) But I know what you mean and personally would prefer this to nesting a using block. – Connell Mar 08 '13 at 09:44
  • @chezy525 Why are you using the typecast in the finally? Is this really needed in .NET 3.5 or later? Before? – Martin Meeser Jun 18 '14 at 09:04
  • @chezy525 Could your finally throw another type cast exception? – Martin Meeser Jun 18 '14 at 09:10
  • 1
    @MartinMeeser, I suppose whether or not you need a typecast in the finally block depends on how you setup your variable (as a `var`, or other base class that isn't required to be an `IDisposable`, it is needed). However, I believe you may be correct that, as it was, the code could throw a typecast exception in the finally block (It should be fixed now). – chezy525 Jun 21 '14 at 13:25
  • 1
    `finally { myObject?.Dispose() }` will be same with `finally { if(myObject == null) myObject.Dispose() }` You don't need to safe cast variable again because you already know that it is instantiated from MyClass. Cheaper one is null check. – uzay95 Jul 26 '18 at 06:38
  • I agree this is much more clear. Thanks for posting. – Brian Birtle May 12 '23 at 07:41
21

It depends. If you are using Windows Communication Foundation (WCF), using(...) { try... } will not work correctly if the proxy in using statement is in exception state, i.e. Disposing this proxy will cause another exception.

Personally, I believe in minimal handling approach, i.e. handle only exception you are aware of at the point of execution. In other word, if you know that the initialization of a variable in using may throw a particular exception, I wrap it with try-catch. Similarly, if within using body something may happen, which is not directly related to the variable in using, then I wrap it with another try for that particular exception. I rarely use Exception in my catches.

But I do like IDisposable and using though so I maybe biased.

Schultz9999
  • 8,717
  • 8
  • 48
  • 87
20

If your catch statement needs to access the variable declared in a using statement, then inside is your only option.

If your catch statement needs the object referenced in the using before it is disposed, then inside is your only option.

If your catch statement takes an action of unknown duration, like displaying a message to the user, and you would like to dispose of your resources before that happens, then outside is your best option.

Whenever I have a scenerio similar to this, the try-catch block is usually in a different method further up the call stack from the using. It is not typical for a method to know how to handle exceptions that occur within it like this.

So my general recomendation is outside—way outside.

private void saveButton_Click(object sender, EventArgs args)
{
    try
    {
        SaveFile(myFile); // The using statement will appear somewhere in here.
    }
    catch (IOException ex)
    {
        MessageBox.Show(ex.Message);
    }
}
Jeffrey L Whitledge
  • 58,241
  • 9
  • 71
  • 99
10

Both are valid syntax. It really comes down to what you want to do: if you want to catch errors relating to creating/disposing the object, use the second. If not, use the first.

Smashery
  • 57,848
  • 30
  • 97
  • 128
8

There is one important thing which I'll call out here: The first one will not catch any exception arising out of calling the MyClass constructor.

Madhur Ahuja
  • 22,211
  • 14
  • 71
  • 124
7

From C# 8.0 on, you can simplify using statements under some conditions to get rid of the nested block, and then it just applies to the enclosing block.

So your two examples can be reduced to:

using var myObject = new MyClass();
try
{
   // something here...
}
catch(Exception ex)
{
   // Handle exception
}

And:

try
{
   using var myObject = new MyClass();
   // something here...
}
catch(Exception ex)
{
   // Handle exception
}

Both of which are pretty clear; and then that reduces the choice between the two to a matter of what you want the scope of the object to be, where you want to handle instantiation errors, and when you want to dispose of it.

Jason C
  • 38,729
  • 14
  • 126
  • 182
1

If the object you are initializing in the Using() block might throw any exception then you should go for the second syntax otherwise both the equally valid.

In my scenario, I had to open a file and I was passing filePath in the constructor of the object which I was initializing in the Using() block and it might throw exception if the filePath is wrong/empty. So in this case, second syntax makes sense.

My sample code :-

try
{
    using (var obj= new MyClass("fileName.extension"))
    {

    }
}
catch(Exception ex)
{
     //Take actions according to the exception.
}
Ankur Arora
  • 194
  • 3
  • 15
1

From C# 8.0, I prefer to use the second one same like this

public class Person : IDisposable
{
    public Person()
    {
        int a = 0;
        int b = Id / a;
    }
    public int Id { get; set; }

    public void Dispose()
    {
    }
}

and then

static void Main(string[] args)
    {

        try
        {
            using var person = new Person();
        }
        catch (Exception ex) when
        (ex.TargetSite.DeclaringType.Name == nameof(Person) &&
        ex.TargetSite.MemberType == System.Reflection.MemberTypes.Constructor)
        {
            Debug.Write("Error Constructor Person");
        }
        catch (Exception ex) when
       (ex.TargetSite.DeclaringType.Name == nameof(Person) &&
       ex.TargetSite.MemberType != System.Reflection.MemberTypes.Constructor)
        {
            Debug.Write("Error Person");
        }
        catch (Exception ex)
        {
            Debug.Write(ex.Message);
        }
        finally
        {
            Debug.Write("finally");
        }
    }
Reza Jenabi
  • 3,884
  • 1
  • 29
  • 34