|
In short, the using statement swallows constructor errors. Since the actual code being executed is a try … finally, why not just use try … finally (or better yet, try… catch … finally) and use your own code for capturing and logging all exception? And given the unpredictability of the GC, scalability is better served by following the principle, “if you create an object, clean it up when done with it”. Relying on the GC and using shortcuts like the using statement are things I consider poor engineering choices in the context of the SDLC. Others may disagree, but I have yet to see a reasoned argument against my approach that ends in better software.
|
|
|
|
|
Aah, ok. I can understand not wanting to use the pattern if you need to catch exceptions or heavily expect that you will need to in the future with that class (if you're gonna have to expand the using into a try...catch anyways might as well go ahead and do it).
That being said, if the exceptions happen in the constructor, they will not be swallowed since object creation happens outside of the try...finally . The only exceptions that would be swallowed are the ones that occur while using the object.
Tested using:
class Test : IDisposable
{
public Test() => throw new NotImplementedException();
public void Dispose() {}
}
using (Test t = new Test())
{
Console.WriteLine("constructor exception swallowed.");
}
|
|
|
|
|
I did a demo on this about 1-1/2 years ago, as my fellow developers didn’t see the harm, either. They did afterwards.
Since in a well designed app, the try-catch-finally is mostly copy and paste, it really does not save any meaningful development time to use the using statement.
Exception handling is a key to reducing dev and QA testing, as well as production troubleshooting. By utilizing the exception’s Data collection, the developer can capture runtime values that are very helpful in diagnosing problems in execution. I have, on many occasions, seen production troubleshooting that would have taken a day or more, shortened to minutes, by smart exception handling. In many production systems, that difference in time can mean thousands to millions in revenue losses avoided by significantly quicker resolution.
Using us a shortcut that alleviates the burden of a developer having to remember to call Dispose(). I’d rather use developers who don’t need such shortcuts.
|
|
|
|
|
I view it as more of an option to reduce code noise in specific circumstances. I don't disagree with your examples but I don't see the path to the conclusion of inherent harm.
Playing devil's advocate on myself, I can see the argument that maybe the class itself won't throw once constructed but something could go terribly wrong with the resource. On the one hand if that's somewhat expected like with a network connection, I can see the merit in saying using would be harmful for the reasons you've stated. On the other hand, if errors in the resource are unrecoverable at this layer, then I would argue using explicitly states your expectation - this code should not throw and if it does something has gone horribly wrong we can't recover from here so let the exception propogate upwards. In this situation the whole point is the finally block, not the exception handling.
I agree it can be misused but non-recovering catch es are often misused by improperly re-throwing and losing stack-trace information. I still see the value in re-throwing though if you want to add explicit information to the exception's data to help with debugging.
|
|
|
|
|
A using block does not swallow exceptions, any more than a try..finally would.
The only time an exception would be lost would be if your Dispose method throws an exception. Whilst that's not entirely unheard of, it's a sign of a poorly implemented class, not a reason to reject the using block outright - especially since you'd have exactly the same problem with a try..finally block.
Code written with a using block is going to be significantly more "correct" than code that eschews it in favour of manual clean-up.
Maybe you should post your demo code to try to convince us.
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
Quote: Code written with a using block is going to be significantly more "correct" than code that eschews it in favour of manual clean-up.
That comes across as more religion than a rational approach.
In order to illustrate my point, I can add the C# code I used for the test (in .NET 6.0) and the MSIL output of both. The unit test for the "using" statement took 28 ms, while the standard use took 18 ms. The MSIL for the "using" statement produced 47 lines of MSIL code, while the standard approach without "using" produced 31 lines of MSIL code.
I kept the IDisposable instance simple for this example.
In order to capture the constructor exception, the using block has to be wrapped with a try-catch.
The results of this test, combined with how I capture exception data and the runtime values associated with the exception (to significantly reduce support costs of the SDLC), is why I do not use the "using" statement in production apps. I do use it where appropriate in proof-of-concept and personal utilities, where support and performance is less of a concern.
IDisposable class:
namespace UsingTest
{
public class DisposableClass : IDisposable
{
private Boolean m_blnDisposeHasBeenCalled = false;
public DisposableClass()
{
Int32 denom = 20;
Int32 numer = 0;
Int32 result = denom / numer;
}
public String WhoIsIt()
{
return $"{Environment.UserDomainName}\\{Environment.UserName} on {Environment.MachineName} from {Environment.CurrentDirectory} on thread ID {Environment.CurrentManagedThreadId.ToString()}.";
}
#region IDisposable Implementation
public void Dispose()
{
try
{
if (!m_blnDisposeHasBeenCalled)
{
Dispose(true);
GC.SuppressFinalize(this);
}
}
catch (Exception exUnhandled)
{
exUnhandled.Data.Add("m_blnDisposeHasBeenCalled", m_blnDisposeHasBeenCalled.ToString());
throw;
}
}
~DisposableClass()
{
Dispose(false);
}
public void Dispose(Boolean disposing)
{
try
{
m_blnDisposeHasBeenCalled = true;
}
catch (Exception exUnhandled)
{
exUnhandled.Data.Add("m_blnDisposeHasBeenCalled", m_blnDisposeHasBeenCalled.ToString());
throw;
}
}
#endregion IDisposable Implementation
}
}
Unit test with the "using" statement
[TestMethod]
public void UsingStatementTest()
{
try
{
using (DisposableClass test = new())
{
try
{
String result = test.WhoIsIt();
}
catch (Exception ex)
{
Assert.Fail($"INNER: {ex.Message}");
}
}
}
catch (Exception exOuter)
{
Assert.Fail($"OUTER: {exOuter.Message}");
}
}
Unit test with a standard approach:
[TestMethod]
public void StandardUsageTest()
{
DisposableClass test = null;
try
{
test = new();
String result = test.WhoIsIt();
}
catch (Exception exOuter)
{
Assert.Fail($"ONLY: {exOuter.Message}");
}
finally
{
test?.Dispose();
}
}
MSIL from the "using" statement text method:
.try
{
IL_0000: newobj instance void [UsingTest]UsingTest.DisposableClass::.ctor()
IL_0005: stloc.0
.try
{
.try
{
IL_0006: ldloc.0
IL_0007: callvirt instance string [UsingTest]UsingTest.DisposableClass::WhoIsIt()
IL_000c: pop
IL_000d: leave.s IL_0027
}
catch [System.Runtime]System.Exception
{
IL_000f: stloc.1
IL_0010: ldstr "INNER: "
IL_0015: ldloc.1
IL_0016: callvirt instance string [System.Runtime]System.Exception::get_Message()
IL_001b: call string [System.Runtime]System.String::Concat(string,
string)
IL_0020: call void [Microsoft.VisualStudio.TestPlatform.TestFramework]Microsoft.VisualStudio.TestTools.UnitTesting.Assert::Fail(string)
IL_0025: leave.s IL_0027
}
IL_0027: leave.s IL_0033
}
finally
{
IL_0029: ldloc.0
IL_002a: brfalse.s IL_0032
IL_002c: ldloc.0
IL_002d: callvirt instance void [System.Runtime]System.IDisposable::Dispose()
IL_0032: endfinally
}
IL_0033: leave.s IL_004d
}
catch [System.Runtime]System.Exception
{
IL_0035: stloc.2
IL_0036: ldstr "OUTER: "
IL_003b: ldloc.2
IL_003c: callvirt instance string [System.Runtime]System.Exception::get_Message()
IL_0041: call string [System.Runtime]System.String::Concat(string,
string)
IL_0046: call void [Microsoft.VisualStudio.TestPlatform.TestFramework]Microsoft.VisualStudio.TestTools.UnitTesting.Assert::Fail(string)
IL_004b: leave.s IL_004d
}
MSIL for the unit test of the standard approach:
.try
{
.try
{
IL_0002: newobj instance void [UsingTest]UsingTest.DisposableClass::.ctor()
IL_0007: stloc.0
IL_0008: ldloc.0
IL_0009: callvirt instance string [UsingTest]UsingTest.DisposableClass::WhoIsIt()
IL_000e: pop
IL_000f: leave.s IL_0033
}
catch [System.Runtime]System.Exception
{
IL_0011: stloc.1
IL_0012: ldstr "ONLY: "
IL_0017: ldloc.1
IL_0018: callvirt instance string [System.Runtime]System.Exception::get_Message()
IL_001d: call string [System.Runtime]System.String::Concat(string,
string)
IL_0022: call void [Microsoft.VisualStudio.TestPlatform.TestFramework]Microsoft.VisualStudio.TestTools.UnitTesting.Assert::Fail(string)
IL_0027: leave.s IL_0033
}
}
finally
{
IL_0029: ldloc.0
IL_002a: brfalse.s IL_0032
IL_002c: ldloc.0
IL_002d: call instance void [UsingTest]UsingTest.DisposableClass::Dispose()
IL_0032: endfinally
}
|
|
|
|
|
MSBassSinger wrote: That comes across as more religion than a rational approach.
Says the person trying to convince me that a using block swallows exceptions, whilst showing no evidence of it doing so.
MSBassSinger wrote: The unit test for the "using" statement took 28 ms, while the standard use took 18 ms.
So code with different behaviour, compiled in a debug build, has approximately 10ms difference in your tests?
Aside from the issue of micro-optimisation - any extra overhead from the using construct will be dwarfed by the cost of your real code - a unit test is not suitable for micro-benchmarks. You need to "warm up" your code, then measure performance over thousands of runs to get a meaningful result. Try using BenchmarkDotNet[^] to measure the code instead.
And while you're at it, fix your tests so that you're comparing the same - or at least comparable - code. Change your "standard" test to:
public void StandardUsageTest()
{
try
{
DisposableClass test = null;
try
{
test = new();
try
{
String result = test.WhoIsIt();
}
catch (Exception ex)
{
Assert.Fail($"INNER: {ex.Message}");
}
}
finally
{
test?.Dispose();
}
}
catch (Exception exOuter)
{
Assert.Fail($"OUTER: {exOuter.Message}");
}
}
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
I am sorry you do not fully understand what I wrote.
First, I did show that constructor exceptions are "swallowed", though I do admit that is a subjective term. To be more precise, constructor exceptions occur outside the MSIL exception handling. In order to catch the constructor exception, the "using" has to be wrapped in a try-catch of its own.
Second, because of what I just explained, the unit test that implements "using" needs the outer try-catch, while the other approach does not. The "using" approach results in nested try-catches, which is why it creates more lines of code to be executed. The coding for the unit tests is correct "as-is". The outer try-catch in your version of the StandardUsageTest() method is unnecessary.
Third, the execution time is not the point. I ran them several times, to account for any caching, and the time relationship remains the same. The point is that a very popular C# coding shortcut results in more lines of code to be executed (MSIL) than the other approach, and forecloses the opportunity to capture runtime values in the exception's Data dictionary within the MSIL code.
If you want to stick with the "using" statement, then do so. I am not trying to tell you or anyone else what they should do. I am presenting information for the "what" and "how" so that an objective person can decide for themselves. I provided the details, as was asked, and it completely and rationally supports my thesis. If you agree, fine. If you disagree with rational reasons you can explain, then educate us all. But since you are disagreeing without a rational basis, that is fine. You should continue to do things the way you think is best. But if you don't like my approach, just admit it is a personal preference to stick with the "using" statement. For example, if benefitting from more advanced exception handling does not provide a value for you, then implementing the "using" statement makes sense.
|
|
|
|
|
MSBassSinger wrote: To be more precise, constructor exceptions occur outside the MSIL exception handling. In order to catch the constructor exception, the "using" has to be wrapped in a try-catch of its own.
OK, that makes more sense now.
However, unless you specifically need different catch blocks for exceptions thrown from constructing the class and exceptions thrown from using the class, you can still get away with a single try..catch block in your code; it just needs to wrap the entire using block.
[TestMethod]
public void UsingStatementTest()
{
try
{
using (DisposableClass test = new())
{
String result = test.WhoIsIt();
}
}
catch (Exception exOuter)
{
Assert.Fail($"ONLY: {exOuter.Message}");
}
}
You'll still have the try..finally from the using block nested within the try..catch block from your own code. But as you can see from the MSIL of your own StandardUsageTest method, a try..catch..finally block is implemented as a try..catch block wrapped in a try..finally block.
The IL, for comparison:
1 .method public hidebysig static
2 void StandardUsageTest () cil managed
3 {
4
5
6 .maxstack 3
7 .locals init (
8 [0] class DisposableClass test,
9 [1] string result,
10 [2] class [System.Private.CoreLib]System.Exception exOuter
11 )
12
13 IL_0000: nop
14 IL_0001: ldnull
15 IL_0002: stloc.0
16 .try
17 {
18 .try
19 {
20 IL_0003: nop
21 IL_0004: newobj instance void DisposableClass::.ctor()
22 IL_0009: stloc.0
23 IL_000a: ldloc.0
24 IL_000b: callvirt instance string DisposableClass::WhoIsIt()
25 IL_0010: stloc.1
26 IL_0011: nop
27 IL_0012: leave.s IL_0034
28 }
29 catch [System.Private.CoreLib]System.Exception
30 {
31 IL_0014: stloc.2
32 IL_0015: nop
33 IL_0016: call class [System.Private.CoreLib]System.IO.TextWriter [System.Console]System.Console::get_Error()
34 IL_001b: ldstr "ONLY: "
35 IL_0020: ldloc.2
36 IL_0021: callvirt instance string [System.Private.CoreLib]System.Exception::get_Message()
37 IL_0026: call string [System.Private.CoreLib]System.String::Concat(string, string)
38 IL_002b: callvirt instance void [System.Private.CoreLib]System.IO.TextWriter::WriteLine(string)
39 IL_0030: nop
40 IL_0031: nop
41 IL_0032: leave.s IL_0034
42 }
43
44
45 IL_0034: leave.s IL_0045
46 }
47 finally
48 {
49 IL_0036: nop
50 IL_0037: ldloc.0
51 IL_0038: brtrue.s IL_003c
52
53 IL_003a: br.s IL_0043
54
55 IL_003c: ldloc.0
56 IL_003d: call instance void DisposableClass::Dispose()
57 IL_0042: nop
58
59 IL_0043: nop
60 IL_0044: endfinally
61 }
62
63 IL_0045: ret
64 }
1 .method public hidebysig static
2 void UsingStatementTest () cil managed
3 {
4
5
6 .maxstack 3
7 .locals init (
8 [0] class DisposableClass test,
9 [1] string result,
10 [2] class [System.Private.CoreLib]System.Exception exOuter
11 )
12
13 IL_0000: nop
14 .try
15 {
16 IL_0001: nop
17 IL_0002: newobj instance void DisposableClass::.ctor()
18 IL_0007: stloc.0
19 .try
20 {
21 IL_0008: nop
22 IL_0009: ldloc.0
23 IL_000a: callvirt instance string DisposableClass::WhoIsIt()
24 IL_000f: stloc.1
25 IL_0010: nop
26 IL_0011: leave.s IL_001e
27 }
28 finally
29 {
30
31 IL_0013: ldloc.0
32 IL_0014: brfalse.s IL_001d
33
34 IL_0016: ldloc.0
35 IL_0017: callvirt instance void [System.Private.CoreLib]System.IDisposable::Dispose()
36 IL_001c: nop
37
38
39 IL_001d: endfinally
40 }
41
42 IL_001e: nop
43 IL_001f: leave.s IL_0041
44 }
45 catch [System.Private.CoreLib]System.Exception
46 {
47 IL_0021: stloc.2
48 IL_0022: nop
49 IL_0023: call class [System.Private.CoreLib]System.IO.TextWriter [System.Console]System.Console::get_Error()
50 IL_0028: ldstr "ONLY: "
51 IL_002d: ldloc.2
52 IL_002e: callvirt instance string [System.Private.CoreLib]System.Exception::get_Message()
53 IL_0033: call string [System.Private.CoreLib]System.String::Concat(string, string)
54 IL_0038: callvirt instance void [System.Private.CoreLib]System.IO.TextWriter::WriteLine(string)
55 IL_003d: nop
56 IL_003e: nop
57 IL_003f: leave.s IL_0041
58 }
59
60 IL_0041: ret
61 }
SharpLab[^]
Practically the same number of lines for both.
The one advantage I can see to your method is that, assuming the constructor hasn't thrown an exception, the instance will be available and not disposed-of in your catch block. With the using block wrapped in a try..catch block, the instance will already have been disposed of, and the variable will be out of scope, by the time you reach the catch block.
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
If it actually matters, then...
Widget w = new Widget();
using ( w ) ...
|
|
|
|
|
Even VS 2022 does this. I hate it & its not the correct way to be writing anything in C#.
|
|
|
|
|
|
Edit: My husband had to explain to me that "DOCTOR OF PHILOSOPHY" is what you write on all doctoral dissertations, regardless. This is what I get for not ever pursuing a degree. You may commence with the public shaming now.
I stumbled upon this on the internet while looking for information on LL(k) parsing
GENERAL CONTEXT-FREE RECOGNITION AND PARSING BASED ON VIABLE PREFIXES
By
D. CLAY WILSON
A DISSERTATION PRESENTED TO THE GRADUATE SCHOOL OF THE UNIVERSITY OF FLORIDA IN PARTIAL FULFILLMENT OF THE REQUIREMENTS FOR THE DEGREE OF DOCTOR OF PHILOSOPHY
UNIVERSITY OF FLORIDA
Philosophers are doing this stuff? Really? This is some heavy math and logic. Now, I can see it from the perspective of demonstrating a grasp of the principles of logic, but this seems like overkill, especially given the math involved! This is something I expect of a Doctor of Computer Science, not a Philosophy PhD.
Real programmers use butterflies
modified 28-Nov-21 12:15pm.
|
|
|
|
|
And no smiley?
(I take for granted that you know the meaning of "PhD".)
|
|
|
|
|
Yes, but it's early so you'll have to forgive my transgression.
moar coffee. (edited)
Real programmers use butterflies
|
|
|
|
|
Wait a minute, are you saying this dissertation is probably for a CS degree?
Real programmers use butterflies
|
|
|
|
|
trønderen wrote: I take for granted that you know the meaning of "PhD".
Piled higher and deeper?
Freedom is the freedom to say that two plus two make four. If that is granted, all else follows.
-- 6079 Smith W.
|
|
|
|
|
"PhD" stands for Doctor of Philosophy and is a generic term used in many disciplines, including Computer Science, for doctoral degrees. Some universities used ScD (Doctor of Science) instead.
|
|
|
|
|
I meant to put PhD after philosophy, not computer science. it's early here and I haven't had enough coffee. I'm fixing that. (Edited since)
Edit: Wait. I think I misunderstood you. Are you saying this dissertation was probably for a Comp Sci doctorate?
Real programmers use butterflies
|
|
|
|
|
Yes, almost certainly for a Comp Sci doctorate.
|
|
|
|
|
Yeah my hubby had to explain how this worked to me.
Real programmers use butterflies
|
|
|
|
|
I'm glad that got sorted so there won't be any confusion when some university calls to give you an honorary one!
modified 29-Nov-21 7:01am.
|
|
|
|
|
There are no philosophers in Florida. The alligators ate most of them, and the rest realized that FL is not the place to live if you are practicing philosophy.
|
|
|
|
|
You receive a Doctorate of Philosophy when presenting new work proving that you can extend science. It is not considered science though when you submit it. It becomes science when it is peer-reviewed and has gone through the other procedures that get it accepted into what is considered the body of science. Until then, your work is considered philosophy.
|
|
|
|
|
Just remember when dealing with thesis papers:
BS => Bull S.
MS => More of the Same
PhD => Piled Higher and Deeper
|
|
|
|
|