- 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
Prefix tree
Admin
It sorta makes sense though. Easter-eggs, anyone?
Admin
If someone did notice this, sorry for the repetition, but there's (IMHO) two WTFs in there. One is the handling of the cases... the other is the seemingly selfmade hash-function.
If I'd come up with this way of solving the problem, I would have done:
but this guy apparently made his own hash-function? Why reuse the wheel when you can get paid to reinvent it!
/E
Admin
I find it incredible that there are so many people who don't understand object orientation and design patterns and how to apply them.
Licky Lindsay has the right idea: this is and ideal candidate for the Command Pattern. See "Design Patterns" by the GoF for details.
Admin
Nope, you're wrong. Maybe I was not clear enough. What is and what is not allowed on a command line is defined within the application itself. Therefore there is no need to check for anymore than the allowed switches. In the case presented these are all unique in the first character, hence checking anymore is not necessary.
You suggest that this is some sort of alternative hashing algorithm. This makes you look stupid, not me.
Other than that I think CL interpretation is not something that should require optimisation, as it is typically done only once per application lifetime.
Admin
This is easy to deal with. Just document that unrecognized arguments result in undefined behaviour :)
Admin
seriously!?
that's a bigger wtf than this code.
Admin
switch statements - that evil that was included in Java even though it's supposed to be a languages that enforces OO.
If they start allowing strings as case arguments, where will that lead us next?
switch statements can be useful on the occasions where there are exactly 3 possible outcomes and no others will
ever emerge, for example, 0, 1 or many (assuming negative numbers is not possible).
The poster who suggested use of some kind of map from string to operations had the right idea. The only thing
missing was the "default" case because there needs to be one, even if it outputs that the parameter is invalid.
As for performance, this is start-up, this is not going to be a performance-critical situation, and even an O(N²) sequence
of if(..) else statements should not become a major performance issue unless N increases dramatically. What it does
become is a maintenance issue - what you might call extensibility.
"Hashes are fast" refers to large collections because lookup time is constant. By "constant" it means that it does not increase
with the size of the collection (although in reality it might do because of collisions).
Admin
You left out FileNotFound.
Admin
This almost good but I think switch case statement only take constants !!
at least in C...
Admin
Well aside from the facts that not all options were included (there was a //etc in there if you didn't notice) and you might want to put in additional options in the future, its still not sufficient to assume the user is going to input a valid option. You need to handle it correctly if the user makes a typo or enters an invalid input.
I believe my phrase was a "poor man's hash code", which is what it is. You are taking a String and getting back a integer type primitive back. Its just a really bad one as it will be full of collisions, which is strange as the hashing function you are replacing is actually quite good.
Except if the application is to be a very quick one that is run over and over again in some sort of batch program.
Admin
Even then.
Case 1: "Run over and over again" means the process is invoked, runs to completion, is invoked, runs to completion... In that case, the process launching overhead will swamp all but the most ridiculously WTFed opt processing.
Case 2: The process is invoked only once, but main is repeatedly called. Unless the process' real work is WTFly small or the opt handling is WTFly slow, running time will swamp opt processing time.
Case 3: The process is invoked only once. Running time does not swamp opt processing time. The opts change on every pass through the loop, so you cannot cache the result of their processing. In this case, we're in the bizarro universe, or at least so far from the case originally presented as to make it an entirely different discussion.
Admin
Might want to start that with:
for(int i=0;i<args[].length; i++){
if (args.length < i || "help".equals(args[i])) {
...
} else if ("single".equals(args[i])) {
...
} else if ("multi".equals(args[i])) {
...
// etc.
}
Not really the way I'd choose to do it myself...That conditional tree would have be iterated in it's entirety for every command line argument.
Admin
No, that is not what design patterns are for. You do not read about a design pattern in a book once and then apply it whenever you see a potential candidate. You need to consider the different needs of each application. There may well be many situations where the command pattern is appropriate, but there are plenty others where it would not be due to considerations such as performance.
There is an old joke that you can often tell what books a developer has recently read by looking at his code and seeing what design patterns he (or she) applies whenever possible. But regurgitating design patterns is not how you are supposed to go about developing software.
Admin
What exactly is "the case originally presented"? Did you receive information about this code that is not posted anywhere? Yes, there are many situations where this would be overkill, but there are also situations where it would be necessary. As of now, we have no additional information about this code. What is it running on? How often does it run? What performance requirements does it need to meet? How many potential options are there total? What is the actual code executing, and how long does it normally run? How optimized is the rest of the code? Without knowing any of these variables, it is impossible to know whether or not it really would be worth it to write the code this way. And just because it looks different from how you would initially approach the problem (especially when the way the vast majority of the people posting here are suggesting it be done wouldn't even compile) doesn't make the code a WTF.
Admin
It's the original WTF. I have no more information than you do--somebody switched on hardcoded string hashes for options processing in main.
Such as?
Right. But I do have enough experience with software development in general to know that a situation that justified that abortion would be a truly pathological beast.
It would have to be a truly bizarre combination of circumstances. If that were the case, I wouldn't expect it to be presented her as just a vanilla main method. I can't completely, 100% rule out that this was a good solution, but Occam's Razor and the duck test are on my side here.
It's not just because it looks different from how I would initially approach it. And the validity of others' suggestions has no bearing on my opinion or its validity.
Admin
I could be missing the point entirely here but hash codes do **NOT** guarantee equality. Despite of the fact that this problem it is very unlikely to come up, there are other strings that have the same hash code as the ones provided by the programmer.
To make sure it is the String that you are looking for, you have to call equals anyway.
Admin
For instance one that absolutely needs to optimize performance for whatever reason. Maybe its running in an environment with limited resources like a mobile phone. Maybe it is a task that is run many, many times. Maybe its a something where a delay could cause massive problems. Maybe all of the above. There are plenty of performance critical apps out there.
And before you say "but parsing command line arguments is a very small part of the application", how do you know that this is the only part of the application that was written this way? Its perfectly possible that the entire application was optimized this way (in fact I would consider it unlikely that this would be the only place they would do something like this). In fact its perfectly possible it was generated by a code generation tool that was designed to optimize things like this in case the code it develops ever runs in a performance critical situation.
Oh, give me a break. "Abortion"? "Pathological beast"? Its not that hard to read it, especially since the comments tell you exactly what is going on. Unless you don't understand what a hash is (in which case how did you become a software engineer), it shouldn't be hard at all to figure out what is going on here.
Admin
Right. Low probability, but you could provide a random garbage string--say, "Mxyzptlk"--and have it match the hashcode of a valid option.
Low probability and minimal "badness" if it does happen (unless the bogus string happens to be a simple misspelling of a diferent valid option), so this isn't my main objection to it. Still bad form though, IMHO.
Admin
Can you provide a specific, concrete example? I stated reasons above why your hypotheticals are extremely unlikely. You haven't addressed any of those.
Obviously, I don't. But main is specifically what was presented here. The app may also be doing bubble sort, and unbuffered byte-by-byte I/O. But I'm not talking about that. I'm talking about the WTF that was presented here.
If some other part of the app does it, I'd still be very skeptical of finding a valid situtaion where it's justified, but there'd be one objection (of many) removed.
Yes. Abortion. Pathological Beast.
Hardcoded hashcodes are brittle, IMHO (despite the algorithm being documented in the API). I didn't say it's difficult to understand. It's just an unnecessarily twisted way to go about it. And yeah, the comments make it easy, but without them, there's no way you can tell by looking at that code what it does. Not that it's difficult--it's impossible. Comments to clarify or elucidate are fine. Comments to decrypt are a sign of a WTF.
Are the ad hominem attacks really necessary? Does questioning my qualifications make you feel more "right"?
Admin
No-one's replied to my brillant code :'(
But like, reflection and annotations are so cool! :D
Admin
Of a program that absolutely needs to optimize performance? Well, no I can't name a specific program but one that does tasks like controlling aircraft or monitoring a nuclear reactor would certainly need optimization. I'm sure there are many more, less obvious examples as well.
But the objection that this is done in the wrong part of the application is dependent on it not being done anywhere else. Since we don't know whether or not this was done in parts of the program where it would be executed over and over again in one run of the application, the argument that this is the wrong place to optimize goes out the window.
Except we don't know what was necessary here. All we know is someone optimized their program's command line argument parsing section. The person who submitted it as a WTF probably did it (if they were anything like most of the people here) because he thought it should just do the switch on the option itself instead of the hashcode.
And how is it 'twisted'? Its not the most intuitive way to do it? A merge sort isn't the most intuitive way to sort (that award would probably go to the bubble sort), but that doesn't mean its 'twisted' to use a merge sort.
I did not make an ad hominem attack, nor did I question your qualifications. If you were referring to the last sentence in my previous post, that was referring to 'you' in the general sense, meaning whoever was maintaining that code and having trouble understanding it (which obviously isn't you-jverd)
Admin
I just came across a very similar phenomenon!
I wanted to see how the language Boo does operator overloading in the source, because it recognises it's own method names for overloads, but not, for instance, C# ones in existing libraries. Well the application isn't that important. Anyhow, it appears that the way it does it is as follows: (C# code)
// HACK: optimization to get to the correct operators faster
// is it worthy?
switch (((int)operatorName[3] << 8) + (int)operatorName[operatorName.Length - 1])
{
case ((int)'A' << 8) + (int)'n': // op_Addition
return op_Addition(lhs, lhsTypeCode, rhs, rhsTypeCode);
case ((int)'S' << 8) + (int)'n': // op_Subtraction
return op_Subtraction(lhs, lhsTypeCode, rhs, rhsTypeCode);
case ((int)'M' << 8) + (int)'y': // op_Multiply
return op_Multiply(lhs, lhsTypeCode, rhs, rhsTypeCode);
hahaha!
Admin
As far as a speed-up, this would probably slow him down. A simple string compare would iterate throught he characters, at the first non-matching character, it would bail. Here, i have to create a hash, so that means i need to visit every character and perform some math to generate the hash. This is probably not faster overall.
Um, it probably is not faster, but it scales better : ) because the hash is computed once, while the comparisons would have to be run N/2 on average, where N is number of possibilities. The larger N, the better the hash performs compared to a straightforward compare.
Just 2 cents : ))
Admin
Because if you rely on String.hashCode(), you're at the mercy of the JDK developers and they could change it on you in the next version. By writing your own, you future-proof your code!
Admin
I actually intended my code to be sort of a WTF in itself. An illustration of how the syntax of Java complicates something that would be much simpler in a language that had closures, (Or even C's pointers-to-functions).
I have used this kind of pattern in real code, although never for something this simple with this few cases, and after a while all the warts required by Java start to grate on you.
Frequently-used idioms in a language should require less typing than less-used ones. In well written, OO java, anonymous inner classes are as common as string concatenation, and one should be able to write them with about as many keystrokes.
Admin
I have seen code like this in the wild. I was hacking on XChat 2 when I found it in outbound.c, line 2070, function cmd_gui().
Admin
Is it because you can't do a switch statement on "String" in Java and the programmer strongly against a if else if structure? I am trying to make sense why anyone would intentionally write code like this.
Admin
That better be sarcasm.
Admin
If this code is in java (looks like it, at least), there is one more bug to account for. The line
will throw a ArrayIndexOutOfBoundException if no parameters are passed to the program.
Admin
nwbrown: Even if your assumption is correct, this code is still completely unacceptable. You simply don't hardcode hash values into the code. If this sort of techniques has to be applied. The the proper way should be
string MULTI_STRING = "multi"
int MULTI_CODE = MULTI_STRING.GetHashCode()
...
switch (arg[0].GetHashCode)
{
case MULTI_CODE
...etc
}
This code would be much cleaner and much less error prone. As someone pointed out, if one day Sun decide to change its string Hashing function to be better, using hardcoded values will simply break down. There is no excuse for writing code like this.
Admin
and the java version could mean anything with that colon. i prefer "in" myself, since anyone can actually read it without looking up the syntax.
Admin
Emergency!
Run that java program!
Oh no, garbage collector took over.
Meltdown... :(
Admin
Well for starters your code won't work because you can't use a non-compile time constant as a case. But yes, they probably should have declared the variables as constants (though again, they would have to be compile-time constants). Though thats far from a front page WTF.
And no, they don't have to worry about Sun changing the hashcode function. For starters, as has been pointed out several times, that is in the API itself and changing it would result in major problems (think about what would happen to anything that serialized a hashcode). And second, they are using their own hashcode method anyways, so it should be independent of the String .hashcode() method.
Admin
Which is why I said they probably shouldn't be using Java, but then the Java fanboys came along claiming its a myth that Java is slower than other languages. But actually, there is work being done on real time JVMs on which you don't have to worry about stuff like that.
Admin
nah, More of:
Emergency!
Run that java program!
Oh no, I missed spilt that parameter and now says it has turn of all engines.:)
Admin
Yeah, because you created it before fool!
Admin
Man, you sure do like to twist people's comments and assign motivations that you're not qualified to judge.
I'd like to correct all the bullshit in your post, but I just don't have that kind of time.
Admin
I couldn't be f**k correcting you, your just not worth it.
Admin
Wow, you seem to have some serious personal problems going on there...
Admin
Might I suggest reading some of the comments?
The reason the programmer did this, is because Java doesn't support strings in switches. That's all. You can either do it this way (ugly and silly IMHO, but It's still kinda funny), or do it in an if-else block, which is basically not that much less ugly.
The real WTF here is that a language like Java doesn't support strings in a switch, it should.Admin
Just reading the comment about the hash algorithm possible changes...
What about the following code ?
public static final int
(...)
public static void main (String args[])
{
// Get a hash value for the first argument
int hash = getHashValue(args[0].toCharArray());
switch(hash)
{
case HELP:
...
break;
case SINGLE:
...
break;
case MULTI:
...
break;
// etc
}
}
more human readable code isn't it ?
Admin
M,
No, the case statements must be on compile time constants. But why would he change the getHashValue() method anyways?
Neels,
How would you propose Java handling String switches without losing the advantages of switches and just making it into an if/else tree?
Admin
I'm (want to learn) curious as to what these advantages are?
Admin
Ding ding ding! You win a prize for noticing that the programmer in question did in fact write his own hash code function whos name I have changed to protect to not really that innocent. The logic behind it? The algorithm in java might change so we should write our own! Comments that werent included in the code snip exlain this brilliant decision and they were included in a followup post on the sidebar thread.
Admin
You should have used a MAC. They're faster and better in every way.
Admin
I skipped over the 3rd page of replies because i HAD to type this out to you guys... it seems as though many of you have never had to do CLAs before... Myself not being a "visual" programmer, everything is Command Line, so having CLAs is very useful. I can think of a handful of cases where fast command lookups is needed (like CGIs, for instance). I'll just paste in one of my code snippets and then get on with what i was going to say.
Module Black_Hole
Const DIRPATH = "c:\Program Files\Apache Software Foundation\Apache2.2\htdocs\"
Sub Main(ByVal cmdArgs() As String
Dim Fname As String = "xxxxxxxxxx.xxxxxxxxxx"
' See if there are any arguments.
If cmdArgs.Length > 0 Then
'For argNum As Integer = 0 To UBound(cmdArgs, 1)
Fname = cmdArgs(0)
'Next argNum
End If
Fname = DIRPATH + Fname
If Not (File.Exists(Fname)) Then
Fname = DIRPATH + "retarded.dat"
End If
Now i know that i hardcoded the directory in, but since this is written for my webserver and only my webserver, it's fine. you'll note that the parts that handle more than one arguement are commented out, because i only accept one argument in this program.
there is a handy way, in .NET to do this, actually. There are things called environment variables, and to save you all some time, the way you used them is similar to (at least in VB.NET)
DIM cl AS Integer = 0
cl = Environment.GetEnvironmentVariables().Item("CONTENT_LENGTH")
For instance. There's all sorts of useful stuff in there to use.
Hashing , at least in the example code snippet block quoted above, makes sense, since the //etc is inside the Switch{ ... } braces. If he has, say, 25 commands, and some of them are undocumented, the only way to find out what commands are available to the would be hacker would be to bruteforce hashes using whatever hash algorithm the original programmer used. It's not genius, it's not stupid, it's not even ugly. I think that the real WTF here is that the OP didn't specify how long the Switch{} block went on for. If it is only 4 items, and all of them are readily available by using %programname% /help then, yah, it's sorta silly. but if there's more than that, and some are hidden (as a few people have pointed out strings dump out in the plaintext EXE files) then this is viable, and i can't think of any better way to mask possible input values from the end-user and would-be hacker.
%programname% /hackpentagon
Captcha: "java"
Admin
If you look back to page 1 of comments, someone went through and explained why switches are "faster", aside from the "not needing to test 45 things to get to the 56th". it had something to do with memory offsets.
Admin
Soooo ... what'll we do when the app has to be localized???
Admin
In blah-de-blah years of programming the only times I've seen a huge case-statement was when it was output by some code-generating algorithm like yacc, bison, or our own command language processor.
In normal programmer written code switch statements usually have far fewer than 20 cases, and if you can measure the performance difference over an if-else chain your basic algorithm if fuxored.