• manifoldronin (unregistered) in reply to anonymous

    You can't really appreciate how much java sucks until you are obligated to write java code every god-damned day to feed your children.

    If you are obligated to write any thing every goddamned day to feed your children, that "any thing" inevitably sucks, java or non-java.

     Another (unrelated) point: IIRC, in Java 7, the switch statement will support string literals as cases.

  • Raven (unregistered) in reply to DisturbedSaint

    That's what we call "failing gracefully".  A feature :-)

  • gl (unregistered) in reply to Its a Feature

    Anonymous:

    Ingenuity is garbage if you can't maintain it.   If switches can't be based on strings, then you use a different technique, such as a series of "if" statements or something.  This code, no matter how ingenious, is total garbage.

     But the example code in the book used a switch statement!  So that's how you HAVE to do it.

     
    When they only tool you have is a hammer every problem looks like a nail.
     

  • sf (unregistered) in reply to Licky Lindsay
    Anonymous:
    That is so old fashioned. Java is about interfaces! And collections!

    public static void  main (String args[])
    {
        HashMap<String,Runnable> m = new HashMap<String,Runnable>();

        m.put("help", new Runnable(){ void run(){ /* code for help goes here */}} );
        m.put("single", new Runnable(){ void run(){ /* code for single goes here */}} );
        m.put("multi", new Runnable(){ void run(){ /* code for multi goes here */}} );
        // etc..
      

        m.get( args[0] ).run();
         
    }

    No, I didn't compile it so I don't know if its correct. You know what I meant to write.

    I like it, exception the run() methods must be public.  I doubt it's more efficient but it's different.  Plus you'll need to do something like this:

            Runnable r = m.get( args[0] );
            if (runnable != null)
                r.run();
            else
                usage();

    Even as a Java developer I agree that not being able to put costant strings in a case statement is lame.  That previous post where the person used enums is the closest you can get, and you can't even use that without Java 5.

  • sf (unregistered) in reply to Licky Lindsay
    Anonymous:
    That is so old fashioned. Java is about interfaces! And collections!

    public static void  main (String args[])
    {
        HashMap<String,Runnable> m = new HashMap<String,Runnable>();

        m.put("help", new Runnable(){ void run(){ /* code for help goes here */}} );
        m.put("single", new Runnable(){ void run(){ /* code for single goes here */}} );
        m.put("multi", new Runnable(){ void run(){ /* code for multi goes here */}} );
        // etc..
      

        m.get( args[0] ).run();
         
    }

    No, I didn't compile it so I don't know if its correct. You know what I meant to write.

    I like it, except the run() methods must be public.  I doubt it's more efficient but it's different.  Plus you'll need to do something like this:

            Runnable r = m.get( args[0] );
            if (runnable != null)
                r.run();
            else
                usage();

    Even as a Java developer I agree that not being able to put costant strings in a case statement is lame.  That previous post where the person used enums is the closest you can get, and you can't even use that without Java 5.

  • Nick (unregistered) in reply to DrCode
    DrCode:
    Anonymous:

    Actually the algorithm is part of the API, and since the String class is final you don't have to worry about some jerk overriding it.  In fact, the values really can't change, or that would defeat the point of the hash (if when you put "myvalue" in the HashTable the hashvalue as 987234, and then when you tried to retrieve it the value was 234523, you would never be able to get it back). 

    The algorithm was different in early versions of Java, and although it is (highly) unlikely, there is no reason why it couldn't change again. If it did, the code would have to be updated. Even now, it would need to have a test to see if it is before version X, and if so use a switch statement with one set of values, otherwise use the current values.

    The values from hashCode() can't change over the lifetime of an object, and the value of a String's hash won't change from one run of a program to the next, but they have changed across different versions of Java. (In an earlier version, the hash algorithm just hashed the first 15 characters, and then every 16th character, for speed, if I recall correctly. This was found to perform badly on strings that all began with the same characters, such as URLs or filenames with the full path.)

    When you find yourself baking all these arbitrary magic numbers into your code, it's time to stop and ask yourself, WTF?

    Well, yeah, the Java language changes from version to version.  Lots of code written in the old days would not even compile with a Java 5 compiler.  And actually, yes there is a very good reason why they wouldn't change it again.  It would break pretty much anything that tried to serialize a HashTable (if they did have it different in Java 1.0, that wouldn't be a concern since there was no serialization back then).

    I'm not saying this isn't a WTF, but rather that the reason its a WTF isn't because it depends on hashcodes not changing, nor is it because it doesn't just use the literal strings in the case statements (which of course Java would not allow).

  • coder (unregistered) in reply to Dmitriy
    Anonymous:
    I wonder what was the incentive to adding the whole hash things? I mean there had to be a reason. Granted, whatever it was it was a bad reason, but still I wish I would know what it was. Maybe he didn't know how to write string literals?

    I wrote similar switch statement once. This was written in C language and the code was supposed to run on embedded system without not enough room for all strings to be stored. So I created trivial hash function, replaced all strings with hash values and I got required functionality without redesigning the hardware.

    Different situation and language, I know, but the switch looks quite familiar to me.

  • woohoo (unregistered) in reply to sf
    Anonymous:
    Anonymous:
    That is so old fashioned. Java is about interfaces! And collections!

    public static void  main (String args[])
    {
        HashMap<String,Runnable> m = new HashMap<String,Runnable>();

        m.put("help", new Runnable(){ void run(){ /* code for help goes here */}} );
        m.put("single", new Runnable(){ void run(){ /* code for single goes here */}} );
        m.put("multi", new Runnable(){ void run(){ /* code for multi goes here */}} );
        // etc..
      

        m.get( args[0] ).run();
         
    }

    No, I didn't compile it so I don't know if its correct. You know what I meant to write.

    I like it, except the run() methods must be public.  I doubt it's more efficient but it's different.  Plus you'll need to do something like this:

            Runnable r = m.get( args[0] );
            if (runnable != null)
                r.run();
            else
                usage();

    Even as a Java developer I agree that not being able to put costant strings in a case statement is lame.  That previous post where the person used enums is the closest you can get, and you can't even use that without Java 5.

    Apart from the fact that Java 7 will allegedly support string literals in "switch"-statements: The "switch"-syntax for enums - see one of the postings above for that, I won't repeat it here - is more elegant and less error-prone (more type safe!) than string-literals - you won't notice a mis-typed string literal, but you will notice a mis-typed enum constant at compile-time.

    If you are using a Java version <1.5, use "if/if else". Why it should be that more desirable to use "switch" is not really clear. Because of the "falltrough"-feature of "case"-branches without "break"? because of "break"? because of the fact that it is more symmetrical ("if" in the first vs. "else if" in all the following branches)? Because of "default"? If these points are bothering you, here's one more subtle way to do it ;o)

            public static void main(String[] args)
            {
                    do
                    {
                            if (args[0].equals("help"))
                            {
                                    ...
                                    break;
                            }
                            if (args[0].equals("single"))
                            {
                                    ...
                                    break;
                            }
                            if (args[0].equals("multi"))
                            {
                                    ...
                                    break;
                            }
                            defaultCase:
                            {
                                    ...
                            }                        
                    }
                    while(false);
            }
    

    Emulating "fallthrough"-behaviour is simply done with "equals() || equals() ...".
    The "do/while" serves to provide a pseudo-loop (it is always only passed once), because arbitrary blocks unfortunately cannot be left with "break". Note that "break" here is not as strictly necessary as it is in "switch" (if there is no "default"), because only one "equals()" will match (provided that you don't repeat a literal), but you loose efficiency, because all the "equals()"-expressions are then always evaluated.
    "defaultCase:" is an arbitrary label which just serves to highlight the signification of the following statement(s) (if present, you do need the "break"s!) Note that you either have to use a braces-block after the label or remove it if no default-statement(s) is(are) present, because a label on it's own is not allowed, whereas a label with an empty braces block afterwards is. So using the block saves you from thinking about this. You could, of course, always use an empty statement in that case (i.e. "defaultCase: ;"), but this is less than elegant. Moreover, the braces-block makes the whole structure more symmetrical and akin the "switch"-statement. One caveat: the label may obviously not be called "default:" (in this exact capitalization) as this is a reserved word.

    captcha: whiskey (with an "e", so it's irish - yummy ;o)

  • Blake Mitchell (unregistered)

    Everyone seems to be missing the whole point of switch statements. Swich isn't there becuase it's a nicer/cleaner way of writing a tree of if/else statements. Switch requires constant numeric case targets because they are used as offsets for the beginning of their code blocks. So that when an argument is provided, the value is the offset the code jumps to, no comparsions take place. This means switch statements are just as fast for one case as for a million. The only cost is the memory needed to store your range of offsets.

  • (cs)

    Maybe I haven't had enough beer yet tonight, but some of these "solutions" that are being presented appear to be as much of a maintenance nightmare as the original WTF.  I like the creativity of the do...while(false) loop, but I seriously doubt the poor guy who inherits that module will have similar feelings.

  • Hank Miller (unregistered)

    That looks a lot like the runtime for the scripting language I have to work with (See stories from me a few weeks back, I was always going to post this, but avoiding that 2000+ line function always seems like a better idea)

    (i is the current line number. data[1] because the scripts are tab delimited, column 0 is for labels, column 1 is for commands....)

    if (strcmp(script_data[i]->data[1],"WINDOW01) == 0) state = m_state_n1 if (strcmp(script_data[i]->data[1],"WINDOW02) == 0) state = m_sate_n2 if (strcmp(script_data[i]->data[1],"WINDOW03) == 0) state = m_state_n2a if (strcmp(script_data[i]->data[1],"WINDOW04") == 0) state = m_state_n2b; if (strcmp(script_data[i]->data[1],"GOTO")==0) state = m_state_n3 ....

    switch state { ....

  • clamboat (unregistered) in reply to James

    Anonymous:
    Heh, don't you know why Java doesn't have a native library to handle argument-parsing?  Remember, Java got its name because it was supposed to run on your coffee machine -- how do you pass arguments to your Krups?

     
    Orly?   I don't know your were joshing but actually Java got its name because we were having the naming meeting in a conference room at 100 Hamilton ave. which was only two blocks from the nearest Peets coffee shop in Palo Alto.   It wasn't unusual for us to walk to Peets every day so coffee was never far from our thoughts.  I supposed Java was would run on your coffee machine, but usually the archetypical Java app was a light switch or thermostat.  In any case the name was not related to cofee machines.

     ob-slashie //Haven't touched Java in at least 10 years
     ////captcha: 1337!

  • (cs)

    There is also a nice alternative option to getopts: http://jakarta.apache.org/commons/cli/

     It also supports automatic generation of "usage", and different type of parsers, default values, descriptions of options etc. Really easy to use too.

     I would also like to comment that there was nothing in the original post that indicated that the standard string hash function was used. In this situation, you could probably set an upper limit of the length of the arguments and use a perfect hash: http://www.burtleburtle.net/bob/hash/perfect.html. No collisions!

    Then you could just use on-the-fly code generation like this: ;)

    public class Test2 {  
        public static void main(String[] args) {
            System.out.println("public class Test3 { \n" +
                               "public static void main(String[] args) {\n" +
                               "switch(Test2.perfectHash(args[0].toLowerCase())) {" +
                               "case " + perfectHash("single") + ":" +
                               "System.out.println(\"single\");" +
                               "break;" +
                               "case " + perfectHash("multi") + ":" +
                               "System.out.println(\"multi\");" +
                               "break;" +
                               "}" +
                               "}" +
                               "}");
        }

        public static int perfectHash(String s) {
            //implement
        }
    }
     
     Then you just run:

    >javac Test2.java

    >java Test2 > Test3.java

    >javac Test3.java

    >java Test3 multi

    multi 

     TADAAA!!
     

  • missing (unregistered) in reply to Rich
    Anonymous:
    Another thing, he's using hashes for the wrong thing.  Hashes are "fast' when you're looking for a specific value in a non-ordered list.  No non-ordered list here, just individual values. 

    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.

    The hash and switch has time complexity O(N+log M), where N is the length of the user supplied string and M is the number of possible options.  An if ladder with string comparisons is o(N+M) and O(N*M).  If there are a large number of options, especially if many share a common prefix, this could be a viable optimization.  But if that happens, I think you have other problems in your application design, and it still doesn't excuse using bare hash values in the code.

  • (cs)

    wow.  that bit of code makes me either want to slit my wrists, or drop the cup, and dive back into the big c of getopt.h world.

     

  • Andrey (unregistered) in reply to DrCode
    DrCode:
    Anonymous:

    This is why I am a C# Dev. It has taken you 20 post thus far and someone hasn't come up with a nondebatable way to iterate over an array:P  In C#

    foreach(string x in args) //...

    Actually, the Java syntax for iterating over an array is more concise than the verbose C# way of doing it:

    for (String s : args) // ... 

    What was your point again?



    Correction: Java 5 has the foreach loop along with a bunch of other extremely useful and long overdue enhancements.  It still doesn't change the fact that the people who had to code in pre 5.0 java had a lot of crap to deal with.  I got lucky, in a way.  When I went to school (every class taught in Java), Java 5 just came out and I had no pre java5 baggage to deal with.  I sure don't envy the people who had to write multi-line incantations to do things like CLI input and output, nevermind something more useful.

  • brendan (unregistered)
    Alex Papadimoulis:

    Today's Code Snippet is from S.C., who shared this in the Side Bar a littler earlier this week ...

    public static void  main (String args[])
    {
        // Get a hash value for the first argument
        int hash = getHashValue(args[0].toCharArray());

        switch(hash)
        {
            case 972346: // The first argument was "help"
                ...
                break;
            case -91058: // The first argument was "single"
                ...
                break;
            case -4830672: // The first argument was "multi"
                ...
                break;
            // etc
        }
    }

    <!-- End: CommunityServer.Discussions.Controls.PostDisplay.TextPost -->

    <meta content="text/html; charset=utf-8" http-equiv="CONTENT-TYPE" /><title></title><meta content="OpenOffice.org 2.0  (Linux)" name="GENERATOR" /><meta content="Brendan" name="AUTHOR" /><meta content="20061216;16015600" name="CREATED" /><meta content="16010101;0" name="CHANGED" />
    
    
    
    
    
    <style type="text/css">
    <!--
    	@page { margin: 20mm }
    	P { margin-bottom: 2.12mm }
    -->
    </style>
    

    There two problems with this code, you guys have not mentions, first, usually a hash code is expressed as a hexadecimal number. Second this will only work if there is no collision between the input and the case statements (i.e the input is "ehlp" may produce the same hash code as "help"). As for the whole algorithm. The guy probably wanted to increase the performance of the program , but instead of optimizing the most used functions, he optimized everything.

  • Andrey (unregistered) in reply to radiantmatrix
    Anonymous:

    Anonymous:

    Correct, this is because java doesnt support switches based on strings. Actually pretty ingenious, even if its very difficult to read.

    Actually, hashes can collide, so it's also a testing nightmare.  Option-processing is NOT something that should be solved on a per-application basis.  The failure to use a pre-existing solution (like, for example, the GNU getopt for Java) is the source of this WTF.

    That particular is package is GPL.  GPL-licensed code may or may have been allowed in the company where this particular code originates.  I do know that at my company, someone took the effort to plaster printed notices in each elevator and break room saying that every piece of open-source code must be approved by a review board before it can be used.  GPL was listed under the "forget it" column.  Chances are, the author of this code decided that importing a 3rd party package was more trouble than throwing together his own implementation (which probably worked well enough to handle the needs of that particular app)

  • aasoft (unregistered)

    Didn't reall all the replies, but...

    there's actually a perfectly valid reason for doing this - iv the application was to be decompiled, then the text strings would be apparent and the decompiler(the person) could find something about about the app. But, given this is java, i dont see a single reason to do so.

     

    Captcha: ranom

    Hmm, i wonder what the hash of that is....

  • Ohnonymous (unregistered) in reply to Andrey

    The REAL WTF is that Java doesn't already have a built-in module like optparse (which has been in Python since 2.3).

  • (cs) in reply to brendan

    Anonymous:
    The guy probably wanted to increase the performance of the program , but instead of optimizing the most used functions, he optimized everything.

    I'm thinking everyone's over thinking this snippet.   I'd bet my $0.02USD that the guy was just trying to come up with a different way of parsing the command line, as long chains of if...elses feel rather clumsy.  I'm pretty sure we've all been guilty of that in our novice years.  Personally, I often find string operations - no matter how applicable - somewhat hacky as I naively consider integer ops more "solid," but I come from a C/C++ environment where I've been smacked in the face with access violations far more times than I'd like to admit.

     Long story short, I think the creator of this WTF is merely guilty of putting more importance on how his code looked in the editor than its functionality.
     

  • - (unregistered) in reply to Josh
    Anonymous:

    This is why I am a C# Dev. It has taken you 20 post thus far and someone hasn't come up with a nondebatable way to iterate over an array:P And why use a language that doesn't give you something really basic like switch statements that can handle strings.

     In C#

     foreach(string x in args)

    You are right that switch statements doesn't support Strings, but Java has an almost identical way of iterating over an array (Since 1.5)

    In Java:

    for(String x: args)

    BTW: In this case, it would be better to use one of the free open source libraries out there that parses command line parameters. Yes, I said free and open source, two words you don't see very often in the .Net world...

    Captcha: craptastic. That's a bit unfair, C# is not a bad language...

  • (cs) in reply to HC

    Anonymous:
    You are right that Java does not support using strings in a switch statement. Why? Dunno. But in case anyone is wondering, what he is trying to accomplish can be done simply by:

    public static final void main(String args[]) { for (int i=0;i

    Ah, the infamous proof by omission. "Reader may easily supply the details." ;) 

    Anonymous:

    I like it, except the run() methods must be public.  I doubt it's more efficient but it's different.  Plus you'll need to do something like this:

            Runnable r = m.get( args[0] );
            if (runnable != null)
                r.run();
            else
                usage();

    I'd probably use:

    try{ m.get( args[x] ).run(); }

    catch(NullPointerException e) { usage(); } 

    But yeah, same diff if it's not a tight loop.

    Anonymous:
    Everyone seems to be missing the whole point of switch statements. Swich isn't there becuase it's a nicer/cleaner way of writing a tree of if/else statements. Switch requires constant numeric case targets because they are used as offsets for the beginning of their code blocks. So that when an argument is provided, the value is the offset the code jumps to, no comparsions take place. This means switch statements are just as fast for one case as for a million. The only cost is the memory needed to store your range of offsets.

    There is such a thing as "syntactic sugar"; languages designed since the late 90's let you switch against all sorts of crazy crap, as long as you define an equality function on your object. Pure integer lookups are still compiled down to the bare minimum, the rest expand to the normal more verbose tests internally.

  • Tomas (unregistered)

    i like this code and using it similary quite often i only use #define for values because i am using no comments

    besides making it nicer and shorter code (case string) it is also obfuscating the strings which are otherwise visible in plain text in exe file

  • Ted (unregistered)

    In addition to the normal WTF-i-ness of this code, isn't arg[0] the command-line that started the process, and arg[1] is "the first argument"?

     
    So it's just just AFU, it's buggy, to boot!
     
    (I am going to steal this design, though :) 

  • (cs) in reply to Ted

    Anonymous:
    In addition to the normal WTF-i-ness of this code, isn't arg[0] the command-line that started the process, and arg[1] is "the first argument"?

    In C/C++, yes, but apparently java doesn't work that way.

    source 

  • C Gomez (unregistered) in reply to InvalidHandleValue

    Using hashes is begging for it to break in a future version of Java if the hashing method changes (I'm prettu sure it is not guaranteed to remain fixed).  That's the mistake.  Other than that, the coder prefers switch() to an if tree... so what.  It doesnt mean the language sucks or that even he sucks.  He was trying to be clever,  but is endangering the forward compatibility of the code.

    This isn't as awful as many of the CSOD's, but in some cases people are picking on it for lame reasons.

  • B (unregistered)

    For parsing a small set of command line options, this is silly because you only parse these options once and it is not like that is going to kill your performance, even if you add as many as your OS would allow to be passed on the command line. Clarity is king.

    But the idea does have some merit in places where performance is an issue.  For instance inside an inner-loop that will have to do the comparisons thousands of times per second. This idea is somewhat related to generic symbol lookup using maps or hashes, but with some differences.  Among other things you can exploit the fact that a case can fall through to the next case, which is sometimes useful for some types of state machines.  Also, the compiler may be able to generate better code than for a you would get if you were using a map.

    You should avoid coding these things by hand, though.  The sensible thing to do is to have the code generated from a specification so that you have an easily understandable representation which is easy to edit and,  so nobody gets tempted to tamper with the implementation.  Another option is to use something already designed for this purpose, like gperf.

    In this example, using a hash-and-switch scheme was excessive for the reasons outlined above.  But those of you who laughed:  what I read into that is that you probably have a bit more to learn since your comments were damning without any qualification as to why and because you obviously do not seem to have read much code.
     

  • B (unregistered) in reply to C Gomez

    "if tree?"

    Yikes, are you one of those horrible people who nest conditionals deeply?  That's just plain evil.
     

  • B (unregistered) in reply to brendan

    The integer doesn't care if the literal form is hexadecimal, decimal or octal.  Where on earth do you get this stupid idea?

  • Thygrrr (unregistered) in reply to theory

    Anonymous:
    I almosted WTFed out loud to this one...

     

    Hey, I actually DID!

     

    Captcha is "clueless", muhahah. 

  • (cs)

    The more I think about this, The Real WTF Here is in most of the comments so far.  Yes, its a bit silly to be this obsessed with performance in what appears to be a Java program (unless the //etc is replacing an unusually large number of conditions) as if performance was that big of an issue you should start out with a faster language.  And yes, the code would have been much more readable had the author just used compile time constants set to each value.  Sure, there is the very slight possibility you might end up with a collision, but that is easy to deal with.  And there is even the slighter possibility that the powers that be in JavaLand decide to change the algorithm, but that probably won't happen as it would wreck havoc with anything that relied on serialized hashcodes. 

    But I've seen much, much worse than a few hard coded constants or too much of an obsession with performance.  Including all these high and mighty people here complaining about other people's code when they obviously know so very little about the subject.  Yes, I understand that switch statements are not that common these days, and thus I don't really expect every programmer out there to be experts in them.  But if that is the case, you probably should think before making fun of someone else's code.

    First, as has been said ad nauseam, you cannot use a java.lang.String in a Java switch statement.  So all you who said the author should have left it in String form without converting it to a hashcode get a big fat F.  And also (as repeated an almost equally high number of times), you cannot use non-compile time constants as cases, so those of you who said he should have had the cases be the more readable "--help".hashcode() get a big fat D.  And for those of you who complain about Java not supporting either of those previous ideas, well you pass for knowing enough about the language to know what will and will not compile, but you still get a C for not understanding the purpose of the switch statement.  It is not just a more readable version of the if/else if block (in fact I would say if/else ifs are much more readable), there is an actual performance increase when using switch statements since the compiler can just skip ahead to the correct case (assuming one exists) and not have to evaluate every statement on the way there.  The only real way to do that with Strings or other objects (other than just replacing the whole thing with if/else ifs internally, which would again defeat the whole point) would have to involve calculating the hashcode (in other words, exactly what this guy is doing right here).

  • brendan (unregistered) in reply to InvalidHandleValue
    InvalidHandleValue:

    Anonymous:
    The guy probably wanted to increase the performance of the program , but instead of optimizing the most used functions, he optimized everything.

    I'm thinking everyone's over thinking this snippet.   I'd bet my $0.02USD that the guy was just trying to come up with a different way of parsing the command line, as long chains of if...elses feel rather clumsy.  I'm pretty sure we've all been guilty of that in our novice years.  Personally, I often find string operations - no matter how applicable - somewhat hacky as I naively consider integer ops more "solid," but I come from a C/C++ environment where I've been smacked in the face with access violations far more times than I'd like to admit.

     Long story short, I think the creator of this WTF is merely guilty of putting more importance on how his code looked in the editor than its functionality.

    So you think that this look better than an if else statement(s).

    If the guy wanted a fast readable (which case statements arn't) cmd parser, why didn't he use reflection (a couple of classes and use of polymorphm (I think that's how you spell it)). For example (Note: I wouldn't use this, as I am happy with using a if-else state).

    class AClass{
        public static class Cmd_help{
            {/*Operations here*/}
        }
        public static class Cmd_single{
            {/*Operations here*/}
        }
        public static class Cmd_multi {
            {/*Operations here*/}
        }
        public static void main(String[] argv){
            try{
                // NOTE: this is only one way of doing this
                Class.forName("AClass$Cmd_"+ argc[0]).newInstance();
            }
            catch (ClassNotFoundException e){
                // Invalid argument
            }
            catch(InstantiationException e){}
            catch(IllegalAccessException e){}
        }
    }

    While reading all of the post so far, non of you have mentioned one of rules when effiecently testing for values:

    When testing for a range of possible values, test for them in their order of probability of occurence, that is, from the most likely down to the least likely.

    From page 83 Program Design 2nd Edition by Peter Juliff.

    It is very unlikly that most likely option selected is not "help".

    I hate this forum software, There are so many bugs and security holes in this forum, that I have lost count. But because this forum software is not open source and can be only run on a windows platform, nothing can be done (power of using a unix based OS).

  • (cs) in reply to nwbrown
    nwbrown:

    The more I think about this, The Real WTF Here is in most of the comments so far.  Yes, its a bit silly to be this obsessed with performance in what appears to be a Java program (unless the //etc is replacing an unusually large number of conditions) as if performance was that big of an issue you should start out with a faster language.

    Right. Becuase languages have the property of being slow or fast, and Java is a slow language. :rolleyes:

     

     

    The only real way to do that with Strings or other objects (other than just replacing the whole thing with if/else ifs internally, which would again defeat the whole point) would have to involve calculating the hashcode (in other words, exactly what this guy is doing right here).

    But then, unless you have perfect hash, the collision resolution will defeat at least some of the purpose of using switch AND will have the added benefit of making code too "clever" and more unwieldy to read and maintain.

     Not that I'm refuting your claim that some of the people doing the WTFing don't really understand what they're WTFing about.

     

  • (cs) in reply to brendan

    Anonymous:
    So you think that this look better than an if else statement(s).

    Why yes, yes I do, especially when the editor displays reserved words in powder blue.  Leisure Suit Blue is even prettier.

     

    I hate this forum software, There are so many bugs and security holes in this forum, that I have lost count. But because this forum software is not open source and can be only run on a windows platform, nothing can be done (power of using a unix based OS).

    lol 

  • Tom Dibble (unregistered) in reply to brendan

    "When testing for a range of possible values, test for them in their order of probability of occurence, that is, from the most likely down to the least likely."


    (not interested in figuring out how to do quotes in this editor)

    That's a great rule, when performance is your #1 consideration and maintenance is not a consideration at all.

    Generally, though, "logical" form is significantly more maintainable.  There may be several "logical" orders to the allowed arguments, including straight alphabetical and simple -> complex.  In most, though, the "help" or "usage" case makes the most sense at the top of the list as it documents the remainder of the list.

    IMHO, I rarely ever hit situations where this "rule" is applicable.  Either the list of options is really short and the order doesn't affect performance significantly, or the list of options is long and since it can't fit on the screen all at once needs a more "logical" order than what one particular developer believes to be the most likely hit options.  The only case where this makes sense is when doing a string comparison on a short list of options inside a tight loop.  As you might guess, I pretty much never hit such a case in my work.


    In any case, my guess as to what happened here:

    The developer wrote his application.

    The customer said, "I like it, but it takes too long to start up!"

    The developer looked at his series of string compares, remembered the admonishment against string compares in performance-critical portions of the application, and decided to "fix" that.


  • (cs) in reply to jverd
    jverd:

    Right. Becuase languages have the property of being slow or fast, and Java is a slow language. :rolleyes:

     

    While it obviously is more complicated that one language being 'fast' and another 'slow', the Java language almost always falls behind other languages like C in benchmarking tests.  Thus if performance is that important to you, Java probably isn't the right choice.

     

    But then, unless you have perfect hash, the collision resolution will defeat at least some of the purpose of using switch AND will have the added benefit of making code too "clever" and more unwieldy to read and maintain.

    Unless it is a completely imperfect hash (meaning every option hashes to the same value), you will get at least some performance boost.  And for the record, the String hash isn't that bad.  Its not like (as previous posters have argued) simply reordering the letters will get you a collision.  For most scenarios, I would imagine each option would hash to an unique value.  And even if there was one or two collisions, you would still be dramatically cutting down on the number of if/else ifs to run through compared to what would happen if you had to go through each possible condition.  Yes, readability would be harmed, but its very possible that would be acceptable if this was part of a batch job that would be run many times.  Writing readable code should not always win out over other considerations like performance.

  • (cs) in reply to brendan
    Anonymous:

    So you think that this look better than an if else statement(s).

    If the guy wanted a fast readable (which case statements arn't) cmd parser, why didn't he use reflection (a couple of classes and use of polymorphm (I think that's how you spell it)). For example (Note: I wouldn't use this, as I am happy with using a if-else state).

    class AClass{
    .... // all kinds of BS
    }

    Most of the time option parsing only needs to set a class variable/property or two and move on with life. That's heavy, unmanageable overhead for option parsing, unless you're doing whole functions worth of work for each - not remotely common. Also not reusable without even more overhead. As a general replacement for a switch (such as interfacing with a badly designed database that uses strings for everything) it might be a good idea.

    While reading all of the post so far, non of you have mentioned one of rules when effiecently testing for values:

    When testing for a range of possible values, test for them in their order of probability of occurence, that is, from the most likely down to the least likely.

    From page 83 Program Design 2nd Edition by Peter Juliff.

    It is very unlikly that most likely option selected is not "help".

    Oh for god's sake, that's the dumbest thing I've read all day. Compared to the nanosecond it takes to get from the first option to help, the JVM startup time will be an eternity. Hell, it's an eternity even on our timescales.

  • Bernhard Hofmann (unregistered)

    Did anyone consider that the programmer may have wanted to ensure that the source didn't include the parameter strings?

  • Fabian (unregistered)

    Okay, hashes are fast, but unnecessary hashes are just overhead. Switches are faster than if-else trees, so not being able to do a switch on a string in Java sucks big time. Based on the three cases presented here I would do a switch on the first character of the input string: no superfluous hashing and no if-else tree, everybody happy (for this version of the program at least)

    Fabian 

  • (cs) in reply to Fabian
    Anonymous:

    Okay, hashes are fast, but unnecessary hashes are just overhead. Switches are faster than if-else trees, so not being able to do a switch on a string in Java sucks big time. Based on the three cases presented here I would do a switch on the first character of the input string: no superfluous hashing and no if-else tree, everybody happy (for this version of the program at least)

    Fabian 

     How exactly do you think the jdk developers should go about implementing a String switch such that it is still faster than an if-else tree?  Your 'first character switch' sounds like a poor man's hashcode, collisions will be very common and readability would be even worse that it currently is.

  • Fabian (unregistered) in reply to nwbrown
    nwbrown:
    Anonymous:

    Okay, hashes are fast, but unnecessary hashes are just overhead. Switches are faster than if-else trees, so not being able to do a switch on a string in Java sucks big time. Based on the three cases presented here I would do a switch on the first character of the input string: no superfluous hashing and no if-else tree, everybody happy (for this version of the program at least)

    Fabian 

     How exactly do you think the jdk developers should go about implementing a String switch such that it is still faster than an if-else tree?  Your 'first character switch' sounds like a poor man's hashcode, collisions will be very common and readability would be even worse that it currently is.

     

    Hence the "for this version of the program" remark... 

  • (cs) in reply to nwbrown
    nwbrown:
    jverd:

    Right. Becuase languages have the property of being slow or fast, and Java is a slow language. :rolleyes:

     

    While it obviously is more complicated that one language being 'fast' and another 'slow', the Java language almost always falls behind other languages like C in benchmarking tests.  Thus if performance is that important to you, Java probably isn't the right choice.

     

    If this app is a benchmark, and it's one where Java is significantly slower than C, then C would be a better choice. But then, if performance is that big a concern, burning the program's logic into a PLA would be faster still. If this is some kind of business app, then I have no reason to believe that writing it in C will make it significantly faster than writing it in Java--especially if it's written by the same programmer that created this WTF.

     

    Unless it is a completely imperfect hash (meaning every option hashes to the same value), you will get at least some performance boost.  And for the record, the String hash isn't that bad.  Its not like (as previous posters have argued) simply reordering the letters will get you a collision.  For most scenarios, I would imagine each option would hash to an unique value.  And even if there was one or two collisions, you would still be dramatically cutting down on the number of if/else ifs to run through compared to what would happen if you had to go through each possible condition.  Yes, readability would be harmed, but its very possible that would be acceptable if this was part of a batch job that would be run many times.  Writing readable code should not always win out over other considerations like performance.

    In this case, yes, with just a few possible String values, it should be easy to find non-colliding strings (as the programmer has apparently done). And yes, large percentage-wise performance gains will probably still be made even if there are some collisions. But anybody would be pretty hard pressed to find a real-world situation where the ugliness of this solution is justified by meaningful performance gains that can't be equaled or approached by other means.

  • (cs)
    public class TheClass {
    	private static void help() {}
    	private static void single() {}
    	private static void multi() {}
    	
    	public static void main(String[] args) {
    		Class yo = TheClass.class;
    		try { yo.getDeclaredMethod(args[0].toLowerCase(), null).invoke(null,null); }
    		catch (Exception e) { /* print usage */ }
    	}
    }
    
  • (cs) in reply to Fabian
    Anonymous:
    nwbrown:
    Anonymous:

    Okay, hashes are fast, but unnecessary hashes are just overhead. Switches are faster than if-else trees, so not being able to do a switch on a string in Java sucks big time. Based on the three cases presented here I would do a switch on the first character of the input string: no superfluous hashing and no if-else tree, everybody happy (for this version of the program at least)

    Fabian 

     How exactly do you think the jdk developers should go about implementing a String switch such that it is still faster than an if-else tree?  Your 'first character switch' sounds like a poor man's hashcode, collisions will be very common and readability would be even worse that it currently is.

     

    Hence the "for this version of the program" remark... 

    In other words you have no idea how to do it.  Which is strange, as you just read probably the most efficient way to do it (taking the hashcode of the string and switching off it), you just wrote it off because you thought it looked ugly.  Yet your "for this version" algorithm would be both less readable and be less efficient.

  • (cs) in reply to jverd
    jverd:
    nwbrown:
    jverd:

    Right. Becuase languages have the property of being slow or fast, and Java is a slow language. :rolleyes:

     

    While it obviously is more complicated that one language being 'fast' and another 'slow', the Java language almost always falls behind other languages like C in benchmarking tests.  Thus if performance is that important to you, Java probably isn't the right choice.

     

    If this app is a benchmark, and it's one where Java is significantly slower than C, then C would be a better choice. But then, if performance is that big a concern, burning the program's logic into a PLA would be faster still. If this is some kind of business app, then I have no reason to believe that writing it in C will make it significantly faster than writing it in Java--especially if it's written by the same programmer that created this WTF.

     

    Unless it is a completely imperfect hash (meaning every option hashes to the same value), you will get at least some performance boost.  And for the record, the String hash isn't that bad.  Its not like (as previous posters have argued) simply reordering the letters will get you a collision.  For most scenarios, I would imagine each option would hash to an unique value.  And even if there was one or two collisions, you would still be dramatically cutting down on the number of if/else ifs to run through compared to what would happen if you had to go through each possible condition.  Yes, readability would be harmed, but its very possible that would be acceptable if this was part of a batch job that would be run many times.  Writing readable code should not always win out over other considerations like performance.

    In this case, yes, with just a few possible String values, it should be easy to find non-colliding strings (as the programmer has apparently done). And yes, large percentage-wise performance gains will probably still be made even if there are some collisions. But anybody would be pretty hard pressed to find a real-world situation where the ugliness of this solution is justified by meaningful performance gains that can't be equaled or approached by other means.

    I think that attitude (you would be hard pressed to find a real-world situation where performance is more important than readability) is very telling for the software engineering field.  I can think of many instances where readability takes a backseat to performance.  Otherwise bubble sorts would be far more common than the more complex merge sort algorithms.  Take a numerical methods course and see some of the crazy algorithms people come up with just to get gains in performance.

  • (cs) in reply to nwbrown
    nwbrown:
    jverd:

    In this case, yes, with just a few possible String values, it should be easy to find non-colliding strings (as the programmer has apparently done). And yes, large percentage-wise performance gains will probably still be made even if there are some collisions. But anybody would be pretty hard pressed to find a real-world situation where the ugliness of this solution is justified by meaningful performance gains that can't be equaled or approached by other means.

    I think that attitude (you would be hard pressed to find a real-world situation where performance is more important than readability) is very telling for the software engineering field.  I can think of many instances where readability takes a backseat to performance.  Otherwise bubble sorts would be far more common than the more complex merge sort algorithms.  Take a numerical methods course and see some of the crazy algorithms people come up with just to get gains in performance.

    I didn't say anything even remotely resembling "you would be hard pressed to find a real-world situation where performance is more important than readability." I don't kow where you got that from. My quote above does not state or imply anything even vaguely close to that.

     

  • (cs) in reply to Halsbrecherisch
    Halsbrecherisch:
    public class TheClass {
    private static void help() {}
    private static void single() {}
    private static void multi() {}

    public static void main(String[] args) {
    Class yo = TheClass.class;
    try { yo.getDeclaredMethod(args[0].toLowerCase(), null).invoke(null,null); }
    catch (Exception e) { /* print usage */ }
    }
    }

    This is actually a pretty nice way to do it, and probably the way I would have done it myself.  The changes I would have made though:

    1) Prepend each method with "cmd" or something for grouping the methods together in your favourite IDE.
    2) Attach a custom @CommandLineMethod annotation or similar to each method (and check for it at runtime) to:
    2.1) Limit the methods that can be called by the user,
    2.2) Allow the developer to loop over the command methods using isAnnotationPresent(CommandLineMethod.class) for an easy way to output the usage list.

    Sounds complicated at first, but it's actually rather elegant and would make readability pretty good (just take notice of each method with the annotation).
    Also you wouldn't need to add a check for each new method/argument you add, nor would you need to add it to the usage list.
    You could even add a String parameter to the annotation to describe the usage.
     

  • (cs) in reply to Samah
    import java.lang.annotation.Retention;
    import java.lang.annotation.RetentionPolicy;
    import java.lang.reflect.Method;
    
    public class TheClass
    {
      @CommandLineMethod("display help")
      private static void help() { /* code here */ }
      
      @CommandLineMethod("run single")
      private static void single() { /* code here */ }
      
      @CommandLineMethod("run multi")
      private static void multi() { /* code here */ }
    
      public static void main( String[] args )
      {
        Class yo = TheClass.class;
        try {
          Method m = yo.getDeclaredMethod(args[0].toLowerCase());
          if(m.isAnnotationPresent(CommandLineMethod.class)) m.invoke(null);
          else throw new IllegalArgumentException();
        }
        catch (Exception e) {
          System.out.println("Usage:");
          for(Method m : yo.getDeclaredMethods()) {
            if(m.isAnnotationPresent(CommandLineMethod.class))
              System.out.printf("  %-8s: %s\n", m.getName(), m.getAnnotation(CommandLineMethod.class).value());
          }
        }
      }
    
      @Retention(RetentionPolicy.RUNTIME)
      public static @interface CommandLineMethod { String value() default "[TODO]"; }
    }
    
  • Bryan Price (unregistered)

    I guess my WTF was I was thinking C, so why were we hashing the program name? 

Leave a comment on “How Not to Parse Command Line Arguments”

Log In or post as a guest

Replying to comment #:

« Return to Article