Comment On Innovations from the Inventor of the "ElseIf"

Hi, at least he knew how to ident properly.

May be a test case to test the no of level for if condition. Is there a limit for nested if in VB?

if you scroll the page up and down fast with your scroll mouse, the vb code looks like a sparrow sitting.

The OK button on the right side, is a MacOSism. (Gnome2-ism too) Well, kind of, it wouldn't say "OK" but a related verb. (generate, save, ..). Not an excuse to do the same on Windows, but it can explain him putting the ok button on the right side.

Wow. I don't even begin to see a reason for this !?!  Wow.

Is it me, or do 99% of WTF stories start off with "...originally written by a highly paid consultant"?  

yes they do, lol

wow... speachless...

is see where he went wrong... why whould you check if a boolean = true?

If optCashReportDay.Value Then 

  DoCashReportDay 

Else 

  If optCashReportWeek.Value Then 

    DoCashReportWeek 

  Else 

    If optCashReportMonth.Value Then 

      DoCashReportMonth 

    Else...

     [edited for space, not because it's wrong [:P](whatever)]

    End If 

  End If 

End If 

laughing all the way to the bank my my 2 cents

I'm embarassed that I actually tried the scroll thing.... (BTW, you're right... It does look like a sparrow).

/BB

Here's one way to make the code more readable, I am not a VB programmer so the code may not be exactly correct.

Select Case True

   Case optCashReportDay.Value 
     DoCashReportDay  
   Case optCashReportWeek.Value 
       DoCashReportWeek  
   Case optCashReportMonth.Value
         DoCashReportMonth

etc.........

End Select

Dave

 

Do you think he did it because the sparrow? ;)

Um, guys? It's not a sparrow.

Scroll up and down real fast and you'll see it.

It's actually quite obscene.

Oh my...I see it!

Eww.

Your case statement is more readable Dave, but isn't the solution for this logic.



All of this should be replaced with a base class for Report and
subclasses for Cash, Bond, Stock, Option, Asset etc each overriding
Report() of the base class with a parameter that dictates the timespan
for the report.

[+o(] That just hurts. I wonder if you could get your hands on a screenshot of this UI?





Truly... W... T... F...

Dave's code wouldn't work, if this was VB.NET and VB.NET is anything
like C#.  The value following the CASE keyword must be constant,
among other issues.

all I can say about this is..   WTF!!!   

The users would be truly wtf'ed after carefully selecting 10 options,
getting back from a cup of coffee and noticing only 1 option got
executed.

When I wrote my initial reply I envisioned the UI as having radio buttons for the various reports so only one would be selectable at a time. If the UI has checkboxes, list boxes or any other way of selecting multiple reports at a time then with some prudent control naming(control arrays) a FOR EACH loop would solve the problem.

Dave

 

:

Um, guys? It's not a sparrow.

Scroll up and down real fast and you'll see it.

It's actually quite obscene.

I think that is exactly what he was doing - ASCII scroll animation in his code.

Either that, or I would love to see his algorithm for a chess game.

Did he at least set it up as a radio button?



Or can you check multiple reports and only get the first one?

rpresser,



I didn't know that about VB.  I suppose they don't have to break
out of a case to prevent fall-through, and they don't have to worry
about only one case block executing either...



I never cease to be amazed that such a WTF-based language can continue to exist. 

For the rest - that wasn't intended as an anti-VB troll, and I know
that atrocities exist in ALL languages, but one has to admit that VB is
rather chock-full of them.

Blue:

For the rest - that wasn't intended as an anti-VB troll, and I know that atrocities exist in ALL languages, but one has to admit that VB is rather chock-full of them.

Blue - appreciate your clarification, but this site would be better if folks would withhold comments generated purely from their disdain of VB as a language. 

How about just criticizing the misuse of the language (the ostensible topic of the WTF), given that no matter how you might dislike VB's design there are right and wrong ways to do something in it?  Seems to me that'd be more constructive.

~Bob (a reluctant VB user)

~bob - Point taken.  Again, for the record, it is quite possible
to create world-class code in VB or nearly any other language.

Never mind the nested IF.  The biggest problem is that the following functions (or perhaps subroutines) exist:

 DoBondReportDay 
 DoBondReportWeek 
 DoBondReportMonth 
 DoBondReportAnnual
 ...

Maybe a single function or two that you pass a date range parameter
would be a *little* more organized and easier?  How much redundant
code do you think there is in each of these functions?

rlewallen:

Your case statement is more readable Dave, but isn't the solution for this logic.

All of this should be replaced with a base class for Report and subclasses for Cash, Bond, Stock, Option, Asset etc each overriding Report() of the base class with a parameter that dictates the timespan for the report.

 

Agreed, if there is time to do this. We all know that there is not always time to refactor a horrible part of code (even when we were the ones who introduced it). I will not comment on how it should have been done this way from the get go because I do not know the vintage of the original code - yes I still have some VB 3 stuff floating around that has not needed to be changed or the time is not there to refactor it.

This screams of a non VBer working in the language, and somewhat inexperienced because of the different routines to run the same report over different time frames. Smells of newbie fresh out of college.

At the bare minimum I would break out the nested ifs into separate groups of nested ifs based upon each report type. Given we are running reports the extra time to run the ifs is minor (my opinion) and the readability would go way up. Changing them to elseifs would spike that further. Refactoring to at least minimize the number of routines called, contral arrays (or the .net equivilent) for each report, or as recommended into a base class/subclass set would be my next steps.

Completely revamping the UI would probably be best (along with the base class route), but users tend to get attached to such things. As such this would actually be the biggest time hit because you would need to do some work with users to determine what would be the best way to handle this screen. As horrible as the screen sounds, this maybe EXACTLY what the users wants & needs.

Wouldn't it look and work more efficient this way (if not used 'case')?
Problem of for loop would be deciding which procedure to call, 
or you could make a function based on index, but its a matter of taste.

If optCashReportDay.Value = True Then DoCashReportDay 
ElseIf optCashReportWeek.Value = True Then DoCashReportWeek 
ElseIf optCashReportMonth.Value = True Then DoCashReportMonth 
ElseIf optCashReportAnnual.Value = True Then DoCashReportAnnual 
...
End If

Gaarrr.  I didn't realize there was a second page of comments until I'd posted my reply above. The continuing discussion on the second page clearly makes the last paragraph of my post both unnecessary and unwarranted.  I apologize...

rpresser:

Gaarrr.  I didn't realize there was a
second page of comments until I'd posted my reply above. The continuing
discussion on the second page clearly makes the last paragraph of my
post both unnecessary and unwarranted.  I apologize...

No worries.  The first part of your message is interesting
though... If only the first matching case executes (which is what I
thought at first, and was asking about), then using a case statement to
solve this is NOT in fact doable - if I chose 5 reports, only 1 would
print!

I'll try to look for a longer nested-if in our bank software.



Apparently it's the tyranny of VB 5 not having Select Case... End Select

Last Post!  Woot!!

with enough forward speed ( to the right )  this code could lift-off.

I mean, with ElseIf it actually would look a bit more reasonable:

If optCashReportDay.Value = True Then

    DoCashReportDay()

ElseIf optCashReportWeek.Value = True Then

    DoCashReportWeek()

ElseIf optCashReportMonth.Value = True Then

    DoCashReportMonth()

ElseIf optCashReportAnnual.Value = True Then

    DoCashReportAnnual()

ElseIf optBondReportDay.Value = True Then

    DoBondReportDay()

ElseIf optBondReportWeek.Value = True Then

    DoBondReportWeek()

ElseIf optBondReportMonth.Value = True Then

    DoBondReportMonth()

ElseIf optBondReportAnnual.Value = True Then

    DoBondReportAnnual()

ElseIf optStockReportDay.Value = True Then

    DoStockReportDay()

ElseIf optStockReportWeek.Value = True Then

    DoStockReportWeek()

ElseIf optStockReportMonth.Value = True Then

    DoStockReportMonth()

ElseIf optStockReportAnnual.Value = True Then

    DoStockReportAnnual()

ElseIf optOptionReportDay.Value = True Then

    DoOptionReportDay()

ElseIf optOptionReportWeek.Value = True Then

    DoOptionReportWeek()

ElseIf optOptionReportMonth.Value = True Then

    DoOptionReportMonth()

ElseIf optOptionReportAnnual.Value = True Then

    DoOptionReportAnnual()

ElseIf optAssetReportDay.Value = True Then

    DoAssetReportDay()

ElseIf optAssetReportWeek.Value = True Then

    DoAssetReportWeek()

ElseIf optAssetReportMonth.Value = True Then

    DoAssetReportMonth()

ElseIf optAssetReportAnnual.Value = True Then

    DoAssetReportAnnual()

Else

    'And so on .... seriously

End If

Clever, clever, but it's "if"s all the way down.