views:

181

answers:

4

In most examples that you find on the web when explicitly not using "using", the pattern looks something like:

  SqlConnection c = new SqlConnection(@"...");
  try {
    c.Open();
  ...
  } finally {
    if (c != null) //<== check for null
      c.Dispose();
  }

If you do use "using" and look at the generated IL code, you can see that it generates the check for null

L_0024: ldloc.1 
L_0025: ldnull 
L_0026: ceq 
L_0028: stloc.s CS$4$0000
L_002a: ldloc.s CS$4$0000
L_002c: brtrue.s L_0035
L_002e: ldloc.1 
L_002f: callvirt instance void [mscorlib]System.IDisposable::Dispose()
L_0034: nop 
L_0035: endfinally 

I understand why the IL gets translated to check for null (doesn't know what you did inside the using block), but if you're using try..finally and you have full control of how the IDisposable object gets used inside the try..finally block, do you really need to check for null? if so, why?

+5  A: 

In your example, the check for null is unnecessary, as c can't be null after explicit construction.

In the following example (similar to what is generated by the using statement), a check for null would of course be necessary:

SqlConnection c = null; 
  try { 
    c = new SqlConnection(@"..."); 
    c.Open(); 
  ... 
  } finally { 
    if (c != null) //<== check for null 
      c.Dispose(); 
  } 

In addition, a check for null would be required in the following case, as you can't be sure CreateConnection won't return null:

SqlConnection c = CreateConnection(...); 
  try { 
    c.Open(); 
  ... 
  } finally { 
    if (c != null) //<== check for null 
      c.Dispose(); 
  } 
Joe
You don't know that the "..." part of his code isn't "c = null;" so the check would be necessary.
Matt Hamilton
@Matt - true, but he presumably knows what's there.
Joe
My feeling is that because he's reading this from code examples, they probably have the "..." too and the authors aren't taking any chances.
Matt Hamilton
I used ... just to eliminate the noise of the connection string, not necessary to illustrate the point
BlackTigerX
+11  A: 

"using" statements can initialize variables with calls other than constructors. For example:

using (Foo f = GetFoo())
{
    ...
}

Here f could easily be null - whereas a constructor call can never1 return null. That's why a using statement checks for nullity. It's not to do with what's inside the block itself, because the using statement preserves the original initial value. If you write:

Stream s;
using (s = File.OpenRead("foo.txt"))
{
    s = null;
}

then the stream will still be disposed. (If the variable is declared in the initializer part of the using statement, it's read-only anyway.)

In your case, as you know that c is non-null before you enter the try block, you don't need to check for null in the finally block unless you're reassigning its value (which I sincerely hope you're not!) within the block.

Now with your current code there is the slight risk that an asynchronous exception could be thrown between the assignment to c and entering the try block - but it's hard to avoid this sort of race condition completely, as there could equally be an asynchronous exception after the constructor has completed but before the value is assigned to c at all. I would suggest that most developers don't need to worry about this sort of thing - asynchronous exceptions tend to be sufficiently "hard" that they'll bring down the process anyway.

Is there any reason you don't want to just use a using statement anyway though? To be honest, I very rarely write my own finally blocks these days...


1 See Marc's answer and weep. Not usually relevant though.

Jon Skeet
yes, there is a reason in this specific case to *not* use a using statement, normally I use using and don't worry about it
BlackTigerX
my specific case either creates an instance of SqlConnection, or takes a live instance and uses it, at the end I have to dispose in case it wasn't a "live instance"
BlackTigerX
"In your case, as you know that c is non-null before you enter the try block, you don't need to check for null in the finally block unless you're reassigning its value (which I sincerely hope you're not!) within the block." this answers my question, thanks!
BlackTigerX
(re edit) lol - but for the record no *sane* code should ever have to worry about this ;-p
Marc Gravell
+2  A: 

Jon's already covered the primary point here... but just a random piece of trivia; your constructor might return null. Not really a common case, and certainly not the real reason that using does this, and you really shouldn't torture your code for this - but it can happen (look for MyFunnyProxy).

Marc Gravell
wouldn't just throw an exception if it wasn't able to create an instance of SqlConnection?
BlackTigerX
check Jon's comment: "whereas a constructor call can never return null"
BlackTigerX
@BlackTigerX: Yeah, I need to edit that :)
Jon Skeet
@BlackTigerX , @Jon seriously, this is just an "FYI". **Don't** torture code just because of this crazy edge case.
Marc Gravell
A: 

Interesting.. I'm using VS2010 and I find that via my custom code snippets (R :) - using blocks rock.

  • In a using block, You can't assign null (or anything for that matter) to the block variable. It results in a compiler error CS1656
  • Next assigning the block var via a factory method, which returns null. In this case, an empty using block intelligently doesn't call Dispose. (Obviously, you would get a NullReferenceException if you attempt to use the block var)
  • Next in a try block, there are no rules, you could assign null to the variable. Hence a null check before Dispose is mandatory in the finally block.
Gishu