4

I'm finding a possible issue in our code base where the developer forgot to wrap the contents of a using statement with curley braces:

        using (TransactionScope transactionScope = new TransactionScope(Transaction.Current))
            try
            {
                RecordUserConnectionCore(context, userName);
                transactionScope.Complete();
            }
            catch
            {
                transactionScope.Dispose();
                throw;
            }

Does the try/catch get executed inside of the using statement? Is the transactionScope properly disposed?

Please note that this question is in regards to wether the try/catch block is executing inside of the using context. I ask because there are no braces surrounding the try/catch code.

Alex Gordon
  • 57,446
  • 287
  • 670
  • 1,062
  • 1
    You don't need try-catch inside transaction to manage completeness of the transaction. – OmG May 12 '17 at 18:20
  • 1
    Possible duplicate of [Using block vs IDisposabe.Dispose()](http://stackoverflow.com/questions/10984336/using-block-vs-idisposabe-dispose) – Fran May 12 '17 at 18:21
  • my question is specifically in regards to usage of the curley braces NOT about how using /idisposable works – Alex Gordon May 12 '17 at 18:22
  • 1
    Should there be braces there to clearly convey what's intended to be part of the using block? IMO, yes, of course. Is the try/catch part of the using block in this circumstance? Yes. And the `Dispose()` call in the catch is redundant. Which means the whole try/catch block is completely meaningless because all it's doing (aside from the unnecessary Dispose() call) is rethrowing the exception. – itsme86 May 12 '17 at 18:33
  • 1
    When there are no curly braces ONLY the next statement will be the body of the block of code. In this case, the body is TRY/CATCH. But in your example you really don't need all that overhead. You are using try/catch to manage the transaction scope and then just rethrowing the original error. You might as well just remove the try catch and put both statements as the body inside the USING. – Sean Lange May 12 '17 at 18:37

2 Answers2

2

After re-reading, I realized that I missed the actual question.

Yes, the try/catch is "within" the using block, even though the using statement doesn't have braces.

If a block doesn't have braces, then implicitly the block only includes the very next statement or block.

So using another example, you could do this:

if(x = 1)
{
    try{}
    catch{}
}

or this

if(x=1)
try{}
catch{}

and both are exactly the same.

For readability, I'll usually add the braces anyway. I think it's a little bit clearer and someone who doesn't know that detail of syntax won't be confused.


Yes, in your code the TransactionScope always gets disposed.

By putting it in a using block and calling Dispose, Dispose gets called twice if there's an exception. It gets called once in the catch block and again with the end of the using block. (That won't throw an exception, but it's unnecessary.)

The using block means that transactionScope already gets disposed even if there's an exception. So you could just do this:

using (TransactionScope transactionScope = new TransactionScope(Transaction.Current))
{
    RecordUserConnectionCore(context, userName);
    transactionScope.Complete();
}
Scott Hannen
  • 27,588
  • 3
  • 45
  • 62
1

The Try/Catch is executing inside of the using block. Your code is exactly the same as if you had the following:

using (TransactionScope transactionScope = new TransactionScope(Transaction.Current))
{
      try
      {
           RecordUserConnectionCore(context, userName);
           transactionScope.Complete();
      }
      catch
      {
          transactionScope.Dispose();
          throw;
      }
}

One of the reasons you can know that this is true is because the

transactionScope.Dispose(); 

line compiles.

JBdev
  • 353
  • 1
  • 10