- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Edit Admin
That's horrible because RegistryKey is an IDisposable object; not only will you run into memory leak issues, the numbers of handles are also limited.
The correct code would be:
Admin
@MaxiTB - MOST IDisposable do not actually keep a reference to an unmanaged object. Therefor implementing IDisposable is the WRONG approach. Also implementing IDisposable does not do anything... it merely requires you write a method named Dispose...the system will NEVER call it automatically... You could write a method call FooMe and call it in all of the places where you would have called Dispose... Further, it must be legal to call Dispose multiple times, but accessing ANY other ember must throw an ObjectDisposedException... and ANY class which has an IDisposable Member must also implement IDisposable...
In 2022 I led a team of 3 developers, took 19 man months!!!! to eliminate incorrect IDisposable from their code, and encapsulate/abstract all of the incorrect implementations in the .Net Framework and NuGet package..
Admin
Add to the first example, I'd be extremely surprised to find a compiler that didn't convert any pure mathematical expression with known values to a constant during compilation. This syntax is common as it clearly indicates the bit position being tested in a bit field. Guess must be fresh out of the college dorm.
Admin
Users who have been instructed to create HTML pages should have been instructed which editor to use. So, still a WTF.
Edit Admin
I was going to comment that the finalizer will dispose it for you (upon GC), but it looks like RegistryKey doesn't have a finalizer? In .NET Framework, there's a Dispose(bool) method, but no finalizer with a Dispose(false) call??? And in .NET Core, they've removed that indirection!
.NET Framework: https://referencesource.microsoft.com/#mscorlib/microsoft/win32/registrykey.cs,7fd1b519e6923ca5
.NET 8: https://github.com/dotnet/runtime/blob/v8.0.0/src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.cs#L128-L163
Admin
"Sometimes, we have a broken world that we can just do our best no navigate"
I guess i'll navigate past the n-t mixup just before "navigate" :^)
Admin
What these submissions - and to be fair, almost everything that ends up on this site - really show is that the world seems to be filled with people doing things that they don't really understand - they're just fudging their way through in a blind panic, randomly changing things until the errors go away.
Has it always been this way or has it just reached a point where it's become the norm rather than the exception?
Edit Admin
I'd argue that it's always been that way. A good rule for life, which I learned from TDWTF, is this: if you show up, do the work, and are halfway competent, you are light years beyond the competition already. The world doesn't need more geniuses, it just needs more competent people.
Admin
#define FEATURE_SENSE_CHAN (1 << 0)
Aaaand now I'm visualizing an anime starring "Feature Sense-chan".
Edit Admin
I think it's because they realized that since they're using a SafeHandle already, there's no need for a finalizer in a class that doesn't manipulate directly an unmanaged resource's lifetime.
Only the SafeHandle class should have a finalizer (that takes care of calling
CloseHandle) since it's the one managing the lifetime of an unmanaged resource.Edit Admin
It has been my experience that most developers work this way. I really appreciate it when I see bad code and it has the same variable names as in the Stack Overflow highest rated answer for a question vaguely related.
I just had an issue this morning where a developer used a pile of global variables in a chuck of code that processes a file. Shockingly, under an uncommon set of circumstances, one of the variables carried over a value from the previous record. We couldn't get it through the developer's head that the actual problem was the global variables, not the specific bug that happened this time.
When I see new feature introduced in my development ecosystem, I can usually see the progression from problem to solution. 90% of the people I interact with didn't consider the original problem and don't see the value of the solution.
For example, C# fairly recently introduced default interface implementations. The first thing I thought was that for my frequently-added-to interfaces, such as database repositories, I can just add an interface method with a default implementation of "throw new NotImplementedException();", and I can avoid compile errors from other consumers of the interface. In the past, I would have had to make a base class that implements everything and have consumers inherit the base class rather than directly implement the interface. The new pattern is less work, less likely for people to get wrong, and looks more correct.
Edit Admin
The point of IDisposable is to dispose unmanaged resources as soon as possible. Finalizers are called by the garbage collector - there is no guarantee when, in fact, if the objects ends up as a gen-2 object, it will only be called when the process gets terminated. The disposable pattern recommends overwriting the finalizer and recommends allowing multiple disposing, but those are not mandatory - in fact many old IDisposable objects in the framework didn't implement the recommended parts of the pattern, most famously the old XML library. When an object is IDisposible, you have to dispose it as a consumer as soon as possible in an exception safe way, period. That is the rule, it doesn't matter what Dispose implements (see https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/using-objects) ;-)
In .net Framework days RegistryKey relied on the Win32 SDK for access to the registry, using the registry handles. Those get cached, so it is critical to release them to allow the cache to flush.
With netcore Registry parts got reimplemented in a open source non-properitary way, so it's implementation is slightly different. But the usage is the same and as mentioned above, it's not about the implementation, IDispose is a contract, you have to correctly dispose the object.
Edit Admin
It doesn't matter how it is implemented. IDispose is a contract, it has to be disposed correctly (see https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/using-objects). It's up to the creator to the component to make use of the contract the way the creator sees fit. As a consumer of the object, it is your responsibility to follow the contract because you don't even know that will be behind Dispose tomorrow.
IDisposable is a contract that exist since .net 1.0 for handling eager cleanup operations. It doesn't matter what the method implements at this point, if the object has an IDisposable, you have to dispose of it correctly as a consumer of said object (see https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/using-objects). It's exactly the same way as you have to free memory after you alloc it in C; those are also just functions, the contract demands of the consumer to free the object as soon as no longer required.
This is incorrect. Dispose SHOULD allow to be called multiple times, but there is no requirement for this and never has been. In fact it is wrong to rely on this. Since netcore it is best practice to do it, but not required (see basic dispose pattern https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/dispose-pattern). There is still a ton of Disposable objects around in various libraries that don't support multi-disposing, especially when it's thin COM+ wrappers.