- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- 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
Admin
I looked at the snippets, and at frist my brain hurt, then it really hurt.
Admin
He's onto something. Dispose an object and repaint the screen. Get your car fixed at the dealership and intake some muffins and coffee. 3 muffins.
Admin
Setting the obj to Nothing DEFINITELY does something. There could be another object that references it - and rather than trying to access a disposed object, it will instead try to access a null object. But perhaps it has a check in it where it will create the object if the reference is null ?
Also, if the object contains references to unmanaged objects - which for some reason the developer didn't get rid of in their Dispose(), setting the reference to null might help the garbage collector collect the object more quickly.
Remy: Setting it to nothing only clears the reference in the procedure's scope. It won't have any impact in the calling scope, so the caller still has a disposed object. And .NET GC is driven by which generation the object ended up in, and I suspect that the object was around long enough to be in G1 or even G2 before getting disposed (if it's a UI element, it's presumably long lived). It is 100% unnecessary. It's not wrong, per se, but I suspect the generated IL will actually skip that line.
Admin
"Hmm, calling DoEvents twice still isn't working. Well, third time's the charm, right?"
Admin
Hmm.... Repaint 3 times.....? I'll bet that 3rd coat of paint really covers a lot of other sins
Admin
Reminds me of the old days in Solaris 2.3... they recommended a "sync; sync; sync" before doing a system shutdown.
Admin
I'm no Visual Basic .NET coder, but wouldn't the fact that Obj is passed ByRef mean that setting it to Nothing inside Disposeof() does clear out the reference in the enclosing scope?
Admin
I remember a co-worker shouting at his stuck computer "DO SOMETHING.... even if it's wrong." This, however, is a case of a programmer apparently responding to the same demand.
Admin
Gah!!!!! It's Quicken! It has to be. Watching that program's GUI repaint itself over and over again makes me shake my head in awe/horror/dismay.
Admin
Three DoEvents? Brilliant! That will allow those slow first-generation Intel onboard graphics subsystems time to redraw!
Admin
Remy, you would normally be correct, but the ByRef means the caller gets the new reference - so the Nothing does get back to the caller.
Admin
The Thread.Sleep() is a bad misconception but a really common one - I see it a lot.
Honestly, if this is the worst in 15000 line files I've seen much much worse.
Admin
Just one was fine, but they they changed hardware and when it was on a faster machine they had to increase it to three.
Admin
One undercoat, two topcoats. Standard for woodwork, less so for GUIs.
Admin
That's what I sort of guessed. Some kind of race condition that's "solved" by repeating it three times.
Or something else, like doing so forces the garbage collector to run or something but only on systems with a specific amount of memory.
Admin
To paraphrase the old metalworker's saying...
"DoEvents() and Paint() make me the programmer I ain't!"
Admin
Remy, the parameter is passed by reference. This means the field in the outside scope is set to null. In turn, this affects if not null checks which were mentioned previously in the code. The whole thing is still a WTF but the set to null ain't it.
Remy: You are correct. I was thinking that VB.Net's ByRef was, well, a reference (and thus, assigning a new value to the local variable only changes the reference for that variable, not the variable in the outer context). It's been a few years since I've had to use it heavily.
Admin
Perhaps update the post to remove (or at least update) the incorrect
also doing the Obj = Nothing which is definitely a holdover from VB6, where you couldn't rely on objects being released automatically. .NET has proper garbage collection, so that's unneccessary.
bit? The multiple comments here show why that is false.Admin
No need for ByRef in that code, all objects that are nullable are always passed by reference in .Net (both VB and C# at least) and that code will do what is expected. There is always a good idea to clean up your disposable variables by disposing and setting reference pointers to Nothing (or null) in .Net since the GC doesn't always release variables that are correctly disposed but still have a pointer especially if the reference is crossed over several instances.
The null-check, the triple call to DoEvents and the missing check for IDisposable is however real WTF's.
Admin
If at first you don't succeed, try and try again. That's why the Application.DoEvents() is called three times.
Admin
You guys realise that DoEvents() won't always repaint the screen right? If you call it 3 times in a row, one of the events may be a repaint, the others are probably mouse-move or something.
Admin
This code has probably been converted from VB6, where it in some cases did make sense to set object variables to Nothing. In VB6, all method arguments were passed ByRef by default, so this may be reason for the explicit "ByRef Obj". As to Doevents(), the developer probably didn't bother to search and replace every DoEvents call with Application.DoEvents(), therefore he created a "powerful" helper function with an identical signature. Yes, I've spent a few years committing similar crimes myself, and I'm not proud of it.
Admin
@xtal256 ...
.Net DoEvents() doesn't process just one pending GUI event per call. It drains the entire GUI event message queue that existed at the moment of call. Hence the name is DoEvents() plural not DoEvent() singular.
So one call is plenty unless you're calling it at a time when A) the user is interacting furiously with the UI, and B) they're going so fast that they can inject a meaningful number of new events while DoEvents() is processing the existing backlog. That latter threshold is tough to beat on any modern system unless you waited waaay too long before calling DoEvents() the first time. That might in very rare circumstances support a second call. But never a third one. And even then the second call is a poor compensation for your original mistake of waiting too long before making the first call.
I'm going to bet the developer, to the limited degree he understood anything at all about dev, didn't understand any of the above.
Admin
For reference types, the variables are references. Variables themselves may be passed by value (the default) or by reference (ByRef / ref). So ByRef/ref are necessary to modify the variable of the calling code; the default behavior is to pass the variable by value, creating a copy of the reference.
In a normal method, setting local variables to null does not help the GC at all. If the method is transformed by the compiler into a state machine (i.e., iterator blocks or async methods), then setting a local variable to null is actually setting a member variable to null, which can make that object eligible for collection sooner. But for normal methods, setting variables to null in an attempt to help the GC doesn't really do anything. In fact, that code is removed by the compiler. See https://blog.stephencleary.com/2010/02/q-should-i-set-variables-to-null-to.html
If there are multiple references to an instance (what I assume you mean by "the reference is crossed over several instances"), then setting the local variable to null doesn't help the GC because the other references still exist anyway.
Admin
au contraire, the object is passed by reference so setting it to null will affect other contexts..
Admin
.Net has value types and ref types. Object (and derivatives) are refs. Passing an object to a method passes a reference to the object. Passing an object byref passes a reference to the reference (C++ would be ** or tracking handle). So the calling function's reference will also be nulled. Other contexts will depend on the dataflow in the call tree.
Admin
" Obj = Nothing which is definitely a holdover from VB6, where you couldn't rely on objects being released automatically."
Let's be honest: Obj=Nothing is a holdover from Visual C++ 6.00, and a generation of c programmers who couldn't get their heads around the idea of a language with automatic memory management, telling their VB6 colleagues "you must always release memory before the object goes out of scope"
I'm slightly unfair: even now many of you won't appreciate that ASP classic often used application-level code that had application-level scope.
Admin
Back in the day, multiple doevents calls handled (badly) the asynchronous nature of Windows events. When you need windows to handle events (like button pushes), then wait for the result of those events, then handle the events that occurred as a consequence, then display the results... sometimes you could kludge it all together by calling doevents a couple of times more.
Admin
Did nobody else notice that the parameter is declared as "Object", which doesn't have a "Dispose" method?
The default VB settings will use late-binding to call the method, if it exists, or throw an exception if it doesn't. But if you turn on "Option Strict", that code won't compile.
The parameter should be declared as "IDisposable" instead.
Admin
I wonder if it was the same guy? https://thedailywtf.com/articles/Do_It!_Do_It!_Do_It!