.Net Core 6 jumps the shark

[Jon McKee]

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 catches 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.

[Richard Deeming]

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

[MSBassSinger]

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

	/// /// Implement the IDisposable.Dispose() method
	/// Developers are supposed to call this method when done with this Object.
	/// There is no guarantee when or if the GC will call it, so 
	/// the developer is responsible to.  GC does NOT clean up unmanaged 
	/// resources, such as COM objects, so we have to clean those up, too.
	/// 
	/// 
	public void Dispose()
	{
		try
		{
			// Check if Dispose has already been called 
			// Only allow the consumer to call it once with effect.
			if (!m\_blnDisposeHasBeenCalled)
			{
				// Call the overridden Dispose method that contains common cleanup code
				// Pass true to indicate that it is called from Dispose
				Dispose(true);

				// Prevent subsequent finalization of this Object. This is not needed 
				// because managed and unmanaged res

[Richard Deeming]

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. :doh:

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

[Phil Benson]

Hmm... I'm pretty sure that AV solutions will go berserk on that....

Who the f*** is General Failure, and why is he reading my harddisk?

[Dave Kreskowiak]

Hehe. Already handled.

Asking questions is a skill CodeProject Forum Guidelines Google: C# How to debug code Seriously, go read these articles.
Dave Kreskowiak

[Phil Benson]

now I'm curious... don't tell me you provide documentation for exceptions / custom rules :^)

Who the f*** is General Failure, and why is he reading my harddisk?

[Dave Kreskowiak]

I wish I could, but I've said as much as I can. You know how it goes.

Asking questions is a skill CodeProject Forum Guidelines Google: C# How to debug code Seriously, go read these articles.
Dave Kreskowiak

[Phil Benson]

*NOW* I am curious..... that sounds a bit dirty and a bit black arts..... ;P

Who the f*** is General Failure, and why is he reading my harddisk?

[Dave Kreskowiak]

:-O :-\

Asking questions is a skill CodeProject Forum Guidelines Google: C# How to debug code Seriously, go read these articles.
Dave Kreskowiak

[Phil Benson]

Seriously, I'm going to have to do a bit of research as my present job involves *preventing* that kind of thing.... :wtf:

Who the f*** is General Failure, and why is he reading my harddisk?

[MSBassSinger]

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.

[Richard Deeming]

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:

.method public hidebysig static
void StandardUsageTest () cil managed
{
// Method begins at RVA 0x2104
// Code size 70 (0x46)
.maxstack 3
.locals init (
[0] class DisposableClass test,
[1] string result,
[2] class [System.Private.CoreLib]System.Exception exOuter
)

IL\_0000: nop
IL\_0001: ldnull
IL\_0002: stloc.0
.try
{
    .try
    {
        IL\_0003: nop
        IL\_0004: newobj instance void DisposableClass::.ctor()
        IL\_0009: stloc.0
        IL\_000a: ldloc.0
        IL\_000b: callvirt instance string DisposableClass::WhoIsIt()
        IL\_0010: stloc.1
        IL\_0011: nop
        IL\_0012: leave.s IL\_0034
    } // end .try
    catch \[System.Private.CoreLib\]System.Exception
    {
        IL\_0014: stloc.2
        IL\_0015: nop
        IL\_0016: call class \[System.Private.CoreLib\]System.IO.TextWriter \[System.Console\]System.Console::get\_Error()
        IL\_001b: ldstr "ONLY: "
        IL\_0020: ldloc.2
        IL\_0021: callvirt instance string \[System.Private.CoreLib\]System.Exception::get\_Me

[PIEBALDconsult]

Ooohhh... Perl...

[PIEBALDconsult]

If it actually matters, then...

Widget w = new Widget();

using ( w ) ...

[jschell]

Dave Kreskowiak wrote:

The C# script would be compiled and executed without generating an .EXE on disk. It would all be in-memory.

Pretty sure you could have done that since C# 1.0. And you can certainly do it now. You create the code. You compile the code into a 'file' which is actually just a hunk of memory. That is the "dll" You then run the code in the "dll"

[Dave Kreskowiak]

Yep, and it was ugly and included certain restrictions on how the code had to be written.

Asking questions is a skill CodeProject Forum Guidelines Google: C# How to debug code Seriously, go read these articles.
Dave Kreskowiak

[jschell]

Dave Kreskowiak wrote:

Yep, and it was ugly and included certain restrictions on how the code had to be written.

I would need more details. How the code and not for example process failures would lead to problems. I have worked on two products in C# that did dynamic code compiling. Certainly no restrictions that ever stopped what I wanted to do or in one case many customers that were using the product to write code, for the actual code. I didn't try to keep it in memory but the dlls were loaded dynamically in both cases. So converting to memory for that part would have been easy. Now the entire process is "ugly" but in both cases there was much of what was done that could not have been done, in a product feature way, that would have removed that requirement. In both cases people tended to get excited and then over use it. I have done the same with java (at least 3 times) and that problem happens with that as well. However that is a process problem not a code problem. So in C# does it have to do with actually saving it to memory?

[Dave Kreskowiak]

You're thinking in technical terms. My issues with the previous ways of doing it are more "customer" issues than anything technical.

Asking questions is a skill CodeProject Forum Guidelines Google: C# How to debug code Seriously, go read these articles.
Dave Kreskowiak