This is the code I work with on a new job



  • I worked in a small company as a tech lead and a c++ developer but due to troubles with paing salary in time. I've said them goodbuy and found new job as a C# senior developer... hmm after two weeks of inactivity (no projects) I was assigned to VB.NET project (dammmnnn... - it's first WTF)... by the way... I have to deal with the following code:

    <FONT color=#0000ff size=2>Protected</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Overridable</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Function</FONT><FONT size=2> releaseObjects() </FONT><FONT color=#0000ff size=2>As</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Integer

    </FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2>Try

    </FONT><FONT size=2>

    l_MyParent = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    oPrimaryKeys = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    oAdditionalInfo = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    l_strAction = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    TbrToolBar = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    MnuToolBar = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    btCut = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    btCopy = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    btPaste = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    </FONT><FONT color=#008000 size=2>'btSpellCheck = Nothing

    </FONT><FONT size=2>

    btSave = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    btRefresh = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    btClose = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    btPrint = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    btExcel = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    btExit = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    btHelp = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2>Catch</FONT><FONT size=2> oException </FONT><FONT color=#0000ff size=2>As</FONT><FONT size=2> Exception

    </FONT><FONT color=#008000 size=2>'Error

    </FONT><FONT size=2>

    generalErrorHandler(oException, </FONT><FONT color=#0000ff size=2>Me</FONT><FONT size=2>.Text, </FONT><FONT color=#800000 size=2>"Error while releasing objects instances."</FONT><FONT size=2>)

    </FONT><FONT color=#0000ff size=2>Return</FONT><FONT size=2> GeneralConstants.APP_ERROR

    </FONT><FONT color=#0000ff size=2>Finally

    </FONT><FONT size=2>

    </FONT><FONT color=#008000 size=2>'Free mem

    </FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2>End</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Try

    </FONT><FONT size=2>

    </FONT><FONT color=#008000 size=2>'Return OK

    </FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2>Return</FONT><FONT size=2> GeneralConstants.SUCCESS

    </FONT><FONT color=#0000ff size=2>End</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Function

    </FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2></FONT> 

    <FONT color=#0000ff size=2></FONT> 

    <FONT color=#0000ff size=2>Protected</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Overrides</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Function</FONT><FONT size=2> releaseObjects() </FONT><FONT color=#0000ff size=2>As</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Integer

    </FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2>Try

    </FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2>MyBase</FONT><FONT size=2>.releaseObjects()

    </FONT><FONT color=#0000ff size=2>If</FONT><FONT size=2> (</FONT><FONT color=#0000ff size=2>Not</FONT><FONT size=2> oSQLData </FONT><FONT color=#0000ff size=2>Is</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Nothing</FONT><FONT size=2>) </FONT><FONT color=#0000ff size=2>Then</FONT><FONT size=2> oSQLData.Close()

    oSQLData = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    myWindowToCreate = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    dtCombo = </FONT><FONT color=#0000ff size=2>Nothing

    </FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2>Catch</FONT><FONT size=2> oException </FONT><FONT color=#0000ff size=2>As</FONT><FONT size=2> Exception

    generalErrorHandler(oException, </FONT><FONT color=#0000ff size=2>Me</FONT><FONT size=2>.Text, </FONT><FONT color=#800000 size=2>"Error while releasing objects instances."</FONT><FONT size=2>)

    </FONT><FONT color=#0000ff size=2>Return</FONT><FONT size=2> GeneralConstants.APP_ERROR

    </FONT><FONT color=#0000ff size=2>End</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Try

    </FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2>Return</FONT><FONT size=2> GeneralConstants.SUCCESS

    </FONT><FONT color=#0000ff size=2>End</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Function</FONT>

    <FONT color=#0000ff size=2></FONT> 

    <FONT color=#0000ff size=2>Guess how many times did I asked myself WTF?

    </FONT>


  • @vvv said:

    I worked in a small company as a tech lead and a c++ developer but due to troubles with paing salary in time. I've said them goodbuy and found new job as a C# senior developer... hmm after two weeks of inactivity (no projects) I was assigned to VB.NET project (dammmnnn... - it's first WTF)... by the way... I have to deal with the following code:<FONT color=#0000ff size=2></FONT>

    <FONT color=#0000ff size=2>Guess how many times did I asked myself WTF?

    </FONT>

    Everytime he set a variable to null, i'd assume.

    I thought you weren't supposed to hack the GC into performing its job.



  • I'm guessing he's an old VB6 dude who didn't quite know how to approach this new try/catch thing. He's more of an onerrorgoto type guy. Since he was used to handling errors for the entire function (or subroutine), he just wrapped an entire function in a try/catch block and handled any exceptions.

    I especially love the helpful comments.



  • @Whiskey Tango Foxtrot? Over. said:

    I'm guessing he's an old VB6 dude who didn't quite know how to approach this new try/catch thing. He's more of an onerrorgoto type guy. Since he was used to handling errors for the entire function (or subroutine), he just wrapped an entire function in a try/catch block and handled any exceptions.

    I especially love the helpful comments.

    Nah, he's forcing the Garbage Collector to do its job... if you set a variable to NOTHING, then it gets removed from the heap or stack or wherever you kids store data these days.

    I was talking to a friend and he said that there's only a few times stuff like this is OK (forcing the GC) like when you want to make damn sure that file.close() closed the file.



  • I have written code that sets class level variables to nothing.  Mostly to work around what seem to be bugs in the way .NET (dont know if it is the CLR or the VB compiler) handles forms.  We have had several cases of forms being disposed but not properly releasing controls (there is a documented bug for this behavior and mainmenu controls), also had problems where, becuase the control is WithEvents, it has an event handler attached to run a piece of code.  That event handler keeps the form alive which keeps the control alive etc...  I tried to reproduce this in a simple app and couldn't but clearing class level references to controls releases the event handlers and it then all works as it should.

    After saying all that, yeah, we have a contractor who keeps writing VB .NET like it was VB6, 'releasing' local variables by assigning nothing, which I think gets removed by the compiler anyway.  No classes, all the code is in modules, half the numeric variables are long, the other half integer. Even uses on error goto still.  Argh!

    ...like when you want to make damn sure file.close closed the file

    Do you also do things like

    c=2
    c=2    'Just in case it missed the first time

    Sorry, slightly off topic.  What on earth were MS thinking when they re-introduced the bloody 'reference an instance of a form by using its name' type construct in .NET 2005?  Its nearly as bad as allowing Option Strict Off!!



  • @GeneWitch said:

    @Whiskey Tango Foxtrot? Over. said:

    I'm guessing he's an old VB6 dude who didn't quite know how to approach this new try/catch thing. He's more of an onerrorgoto type guy. Since he was used to handling errors for the entire function (or subroutine), he just wrapped an entire function in a try/catch block and handled any exceptions.

    I especially love the helpful comments.

    Nah, he's forcing the Garbage Collector to do its job... if you set a variable to NOTHING, then it gets removed from the heap or stack or wherever you kids store data these days.

    I was talking to a friend and he said that there's only a few times stuff like this is OK (forcing the GC) like when you want to make damn sure that file.close() closed the file.

     That's where .Dispose() comes in handy.  If you run fxCop on your files in VS2005, the thing practically holds your hand through a textbook implementation.



  • @piptheGeek said:

    I have written code that sets class level variables to nothing.  Mostly to work around what seem to be bugs in the way .NET (dont know if it is the CLR or the VB compiler) handles forms.  We have had several cases of forms being disposed but not properly releasing controls (there is a documented bug for this behavior and mainmenu controls), also had problems where, becuase the control is WithEvents, it has an event handler attached to run a piece of code.  That event handler keeps the form alive which keeps the control alive etc...  I tried to reproduce this in a simple app and couldn't but clearing class level references to controls releases the event handlers and it then all works as it should.

    After saying all that, yeah, we have a contractor who keeps writing VB .NET like it was VB6, 'releasing' local variables by assigning nothing, which I think gets removed by the compiler anyway.  No classes, all the code is in modules, half the numeric variables are long, the other half integer. Even uses on error goto still.  Argh!

    ...like when you want to make damn sure file.close closed the file

    Do you also do things like

    c=2
    c=2    'Just in case it missed the first time

    Sorry, slightly off topic.  What on earth were MS thinking when they re-introduced the bloody 'reference an instance of a form by using its name' type construct in .NET 2005?  Its nearly as bad as allowing Option Strict Off!!

    No, i don't do anything like that, and i rarely need to have a file opened once, and then reopened by another method; so i wouldn't use whatever methods he was talking about. I've very poor at parroting what people say, especially when we switch topics so fast (he wasn't here when i typed that, but was here when i was reading the forums).

     



  • @GeneWitch said:

    Nah, he's forcing the Garbage Collector to do its job... if you set a variable to NOTHING, then it gets removed from the heap or stack or wherever you kids store data these days.

    I was talking to a friend and he said that there's only a few times stuff like this is OK (forcing the GC) like when you want to make damn sure that file.close() closed the file.

    Oh, so *that's* what he thinks he's doing? Wow, that is a WTF. I honestly didn't do much more than glance at the method names -- which on hindsight would have helped a bit -- I just noticed that each method was wrapped in a try/catch block, and figured he didn't know what exceptions really were.

     

    vvv's coworker should go look into how garbage collection schemes work, 'specially in .NET. Setting a variable to Nothing (null in C#) does not force the garbage collector to do a thing, it just reduces the reference count for the object that variable referenced. When the garbage collector runs (if it runs for that execution, which isn't always the case) it will reclaim the object if there are no more items referencing it. The only way to ensure that any resources used by a particular item are released is to use the Disposer pattern. And, even then, the only time you ever need to *worry* about whether or not you're releasing resources is if you're using unmanaged resources. Given that they're all Win.Forms objects and strings, and he's likely deriving from the Form object (and the Disposer pattern is thus built in and handled automatically) he didn't have to do a thing.

     

    Also, vvv -- please go shoot your co-worker for inflicting that stupid pseudo-hungarian notation on you and (vicariously) us.



  • @Whiskey Tango Foxtrot? Over. said:

    When the garbage collector runs (if it runs for that execution, which isn't always the case) it will reclaim the object if there are no more items referencing it.

    I'm confident that the GC does a better job than that. A GC must also release unreachable circular-referencing groups of objects. 



  • @ammoQ said:

    @Whiskey Tango Foxtrot? Over. said:

    When the garbage collector runs (if it runs for that execution, which isn't always the case) it will reclaim the object if there are no more items referencing it.

    I'm confident that the GC does a better job than that. A GC must also release unreachable circular-referencing groups of objects. 

    Pardon me for dumbing it down a bit. :)



  • Oh man, In 4 years of programming .Net, I never realised that setting an object reference to null could throw an exception. I am going to have to review a LOT of code!



  • @Whiskey Tango Foxtrot? Over. said:

    @ammoQ said:
    @Whiskey Tango Foxtrot? Over. said:

    When the garbage collector runs (if it runs for that execution, which isn't always the case) it will reclaim the object if there are no more items referencing it.

    I'm confident that the GC does a better job than that. A GC must also release unreachable circular-referencing groups of objects. 

    Pardon me for dumbing it down a bit. :)

     


    You shouldn't dumb things down if it makes them wrong.  Then people think that things are easy and they go write WTF code.   Garbage collection is a very complex topic and it much more than just reference counting.  PS.  hardly any modern garbage collectors use reference counting since it has problems with circular references and requires a lot of overhead with method calls and rolling back the stack.  Mark and sweep is pretty straight-forward and will result in much better performance.  Although that requires that you stop execution of the program.


Log in to reply