- 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
Admin
It is probably a military system, or an hospital system. The redundancy is needed for it to be failsafe.
Admin
Isn't it the purpose of the "select case" construct?
Giving you the instruments to indent some code a bit farter to right :D.... not useful but cool :D
Admin
This is really just an optimization. He won't have evaluate all of those expensive ifs if the case doesn't match.
But in all truth, this make my head ache.
Admin
When I used to work in the Department of Information Technology Redundancy Department IT Section, this kind of coding was encouraged. Other examples include:
while(condition)
{
if(condition)
break;
else
{
DoStuff();
}
}
and
if(condition)
DoSomething();
else if(!condition)
DoSomethingElse();
You just always want to have your bases covered.
Admin
looks like a general handler for { check, radio, combo, select } control types, with additional handling for 'select' and 'radio'... it's possible the developer's intention was to eventually add the general handling code for these four control types at some point in the future, or that the code has been "maintained"...
Admin
It's sometimes difficult to ascertain sarcasm on the internet, so if that was your intention I apologize.
With that said, your statement has to be true. It is a general handler for those control types, but the implementation is stupid and redundant. Instead of picking one huge case with all of those types, there should have been seperate cases for each.
Admin
This looks like one of those things where a bit of refactoring was going on, but the person doing it was half asleep and/or the time was 16:59 and they finish at 17:00 and/or they get pulled into a meeting halfway through doing it. Then, the next day they move onto something else and completely forget to finish off the refactoring they were doing.
Admin
I really like that an uppercase control type will pass the 'switch' and fail on the 'if' test.
Admin
Come on, guys, this one is obvious.
It's simply using CASE to evaluate ControlType regardless of its case, then using IF to handle lowercase values within the CASE, and in case the case of ControlType is not lowercase (perhaps uppercase or a mix of cases) it is handled outside of the CASE or excluded by the CASE statement.
Just in case you can't figure it out ...
Admin
Actually no it won't. When he makes the LCase() call in the "Select Case" line, it will compare the lower-cased version of the control type, while not actually changing the case of the variable. I'm surprised that this guy would know to do that, but didn't have enough smarts to just remove the Select structure altogether.
If StrComp(controlType, "select", vbTextCompare) = 0 Then
ElseIf StrComp(controlType, "radio", vbTextCompare) = 0 Then
....
End If
Admin
Dammit I'm a retard. Sorry, you are totally right. Still, my StrComp solution would work just fine.
Admin
If this is the case, then the WTF is that it is missing a comment.
Admin
Looks like someone told a n00b to use select/case, not else if....so he did.
Admin
This is simply a case of self-documenting code. The case statement tells you what is happening in this block of code. Without it, you would have to read the entire block of code to find out what values of controlType are covered. Sure, the computer has to compare the string one extra time, but it only slows it down by a microsecond. It would take me a few seconds to read the entire block of code!
So, really, this is good code. Too bad about that Visual Basic though.
Admin
Well, in that case, case closed![:P]
Admin
I wonder if there is a lurker on this board working on a vblint, rubbing his hands together with glee every time a new vb post goes up.
Admin
All your base are belong to us.<p>
Sorry, couldn't help myself.<p>
sacha
Admin
vblint?
Wouldn't that just rock like a one-string ukulele...
Admin
Huh, this seems pretty reasonable to me. Whats the problem? There was some ** general code ** for all 4 types snipped out, and the 2 if's handle the specific sections just for those controls in addition to the general code. Get it?
case ** general case **:
if (specific condition) do something specific
if (specific condition) do something specific
** general code **
break;
Admin
Where do you see this ** general code ** snipped out? I only see 'ED Snip comments inside the If statements, none outside. Your statement would only make sense if Alex has posted:
Admin
I don't know much about VB, but isn't there an equivalent of
case default: // do stuff break;Wouldn't that be where you put so called "general stuff"?
To me this just looks like what happens when someone tells an intern "Obviously you'd want to use a
switchfor that." And this is what they come up with.Admin
This looks like a minor WTF and more of a logic error than anything else.
Yes, he forgets to compare the inner control types in a Case Insensitive manner, but that's more of a logic error than anything else. Nobody here can say they have never created a logic error before (with any honesty anyway).
Now, that said, sure the code is redundant. However, it does allow for the possibility of all four control types to share one section of code while allowing for customization of specific types.
Notice that he only shows two types specifically mentioned in the If statement, while he covers four in the case. This tells me that it's likely code that changed from when it was originally written (or he wrote it for something that never came into play). I'd say this is simply a case of stumbling upon code that was maintained.
Sure, as it's written, aside from the logic error, it's fairly silly (and only really requires one extra if check, so it's not exactly killing performance). I would only write somehting like this IF I was planning on handling all those types with the same logic and had a few exceptions crop up. It could be handled by doing what he did or calling a function in each and every case. This is actually better self-documenting and looks cleaner.
Sorry, it's a minor WTF at best.
Admin
Boy, I hope this is a joke.
Admin
As an employee of a leading company that makes hospital systems, no, we don't do that.
We never, never, ever do that.
Admin
however anyone with half a clue would realise that to do that 'switch' you need to do the comparisons anyway. Think about what you just said, "oh if none of those match then he won't need to do any of those expensive if statements", what do you think a switch/case statement looks like once 'compiled'?
Admin
...as opposed to just putting in another indent (with a tab or two) and/or surrounding comments?
Admin
I agree because whether or not this is a "self-documenting" code it's still the wrong use of a case statement. Correct me if I'm wrong, but case statements are optimized for checking more than three (3) conditions. The "if ... else" statement could have just been used instead.
And, seriously, I think the person who wrote the code didn't fully understand the proper use of a case statement.
Admin
As an employee of another leading company that makes hospital systems with the development done in India (BTW, I love losing control of the application I've developed for the last 7 years), you'll never know what wackiness they'll come up with next. [:P]
Admin
rm *.vbs
Admin
Darn! I knew I should have put in the sarcasm tags!
Admin
Is it really that hard to recognize sarcasm when it is directly in front of ones face? If so, it was sarcasm. There, I feel better now!
Admin
Yes, there is an equivalent for the code you posted. But it is for non-covered possibilities. Select 'breaks' after a Case statement. Hence, it would never get to the 'case default'. General code goes outside of the Select statement. Is it so difficult to write a function that does the common stuff?
Admin
I love all the comments about redundant checking to achieve some kind of mythical fail-safety which seem to crop up in every thread in which ridiculous code practices are employed. In most cases, the only "failure" which would cause the checking to be useful would be a failure in the laws of boolean logic, which I think we're pretty safe with nowadays.
I'm afraid that in my opinion, redundant checking simply shows a poor design methodology. Let's not forget that we're in the business of software engineering - our goal is not just to throw something together until it works, but create an elegant, maintainable and readable solution to a problem. That's the fun of doing it! Given that, in all probability, several people will have to read and maintain this code over the years of its useful life, anything which makes them unnecessarily question the code ("why is that extra if statement in there? Hmm, maybe it's something I don't know about
Anyway, this code has nothing to do with that, it's is a simple case of someone not understanding what a select..case statement does. For anyone that's inclined to give the coder the benefit of the doubt... well, I've seen enough people's code to realise that that's rarely the correct option.
Admin
"It's simply using CASE to evaluate ControlType regardless of its case, then using IF to handle lowercase values within the CASE, and in case the case of ControlType is not lowercase (perhaps uppercase or a mix of cases) it is handled outside of the CASE or excluded by the CASE statement."
No, actually - the select converts the value to lowercase, so it will always pass that case. The WTF is that the select statement is superfluous - the kind of error that is common in refactoring.
If the select statement is there to tell you what controls can be selected, then a comment should have been used.
This reminds me so much of Windows Installer/MSI custom action scripts...
Admin
Yay!!! That post gave me my first blue ball. bounce bounce
Admin
To All Those "Base-Coverers":
No No No!
if you need an if, use an if; if you need a case, use a case. To say you should use an if inside the case statement "just to cover your bases" makes me wonder whether you should even be working in the "vodoo-wicky" field of computers and programming.
PS: sorry if i offended anyone, but if we start to question the validity of functions like "case" then we need to start questioning the validity of mathematical functions like addition as well.
Admin
Actually, Rick is right. The 'if' test doesn't lcase() the control name, so 'SELECT' = 'select' will fail.
Admin
When the programmer started, he thought that "check", "radio", "combo" and "select" would have something in common, so he planned to make a common handler for these four cases.
Then he found out that this doesn't work out (for some reason),
so he made the IFs to handle each case on it's own,
but never cleaned out the now useless SELECT; neighter did he replace the IFs by more elegant CASEs.
Admin
Since you're quoting me, I will respond. I know that I didn't mark up my post with sarcasm tags, but I thought it would be obvious by the first sentence in my post. The code is redundant and unnecessary.
Admin
So did the statement that began with "But in all truth, this make my head ache" not tip you off to the fact that it is sarcasm?
Seriously, a lot of posters on this board need to turn their sarcasm radar back on. I think too many people want to show off how smart they are, and will take any chance they can to do it.
Admin
No, Ytram I wasn't talking about your post.
Sorry for the confusion.
Admin
It appears you are assuming that select gets compiled into a long if/else sequence. This might be true, but there is no reason to assume so. I can think of other ways to do it, and they might be faster for some situations.
Select COULD build a state machine, checking each letter for all currently valid combinations - each letter can be an index into an array of function pointers. If there are many cases where this case won't match, only 4/26 will need to compare two letters, while the standard if/else will need to compare 4 letters just to see that it matches none.
Another obvious optimization to select it to build a hash table for all possible values. This might be more CPU time than a 4 place if/else sequence, but if there are many other cases it could be a win.
A smart compiler could recognize a long if/else sequence and do either of my suggested optimiztions (and perhaps more that I have not thought of). In the real world such optimizations are hard to recognize. I don't think any compiler does that, but one could.
That still doesn't explain why you don't have a different case for each option. (Unless there is a lot of common code two all four that Alex sniped without telling us, but if that is the case there should be a function call or something to handle the common code)
Of course I'd be surprised if visual basic was smart enough to actually do a select as anything other than a long if/else. For that matter I'd be surprised if many real world select statements have enough different cases, and are used enough that the cost in extra code is worth the code bloat.
Come to think of it, the most common way to do select in python is a hash table of fucntion pointers.
Admin
I got curious about the topic, so I decided to check out the IL in a .NET application that used a switch case statement. The first one I tried was a 10 case switch statement. The resulting IL was in fact using a Hashtable to determine what action to take. After that I tried using just 3 cases, and it was not using a Hashtable. I tried it up to 10 again, and it appears 10 is the breaking point in .NET, where at 10 cases it will begin using a Hashtable.
When the number of cases is less than 10(and when the switch is being done on a string variable), a call to string.IsInterned is done with the switch variable. This is an optimization that will help to avoid all of the cases if the switch variable is also not in the "intern pool". The intern pool contains all of the string literals(the case literals in this situation), so the call to string.IsInterned will return null if the switch variable is not one of the cases in the switch statement. Beyond that, it looks to simply convert it into an "if-else-if" structure.
Of coure, all of this applies only to the .NET compiler and IL, so it'll most likely be different for your compiler of choice.
Admin
As a military program, redundancy support is provided through additional software/hardware/personnel support. WTF code like this is totally unacceptable.
Admin
using System;
namespace vblint
{
/// <summary>
/// This is a checker for VB code
/// </summary>
class vblint
{
[STAThread]
static void Main(string[] args)
{
System.Console.WriteLine("Warning : You appear to be using VB");
}
}
}
Admin
Unless Option Compare Text is on.
Admin
No, all your Case are belong to us.
Is there any common code between the cases here, or is it all inside ifs?
Admin
This code is fast. If it fails the case check it will jump over the if statements.
If controlType = "Check" it will jump into the case statement but all the If's will fail.
Or he forgot all the lcase's in the if lines.
I wonder how he implemented the case else, else if construction.
Admin
Then why would he be using LCASE() in the CASE?
Admin
That's assuming the author knows what s/he is doing. [;)] This is a WTF, so you can't assume anything...