Comment On How Not to Parse Command Line Arguments

I think the coder needs some 972346.

I almosted WTFed out loud to this one...

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'm not sure I'm understanding this as I don't sepak Java, so could someone please expalin?

Is the WTF the nightmare case values, or does the hashing code simply not work? 

 

CAPTCHA - java, lol 

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 think the programmer probablly heard somewhere that "Hashes are fast" and wanted to optimize what was obviously the application's bottleneck.

This was likely done because the language didn't support using strings as the value in a in a switch/case statement. Highly frustrating for a language not to support, IMO.

Not saying this solution is a good idea, however. 

At least the programmer put in comments...

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

 

 

 

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

 

 

if you think that's genious then you might want to watch out for your own code on this here website thing. 

Brilliant!

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
    }
}

Just Brillant!

I'm adding this to the mission-critical app I maintain, but I'm leaving out the comments and obfuscating what each case handles.  Perfect job security! 

No this would have been the slightly ingenious version that might not have gotten you fired (although you should still be refactored at the earliest possible chance):

 

   int hash = getHashValue(args[0].toCharArray());

    switch(hash)
    {
        case getHashValue("help".toCharArray()):
            ...
            break;
        case getHashValue("single".toCharArray()):
            ...
            break;
        case getHashValue("multi".toCharArray()):
            ...
            break;
        // etc
    }

 

tster:

The pig go. Go is to the fountain. The pig put foot. Grunt. Foot in
what? ketchup. The dove fly. Fly is in sky. The dove drop something.
The something on the pig. The pig disgusting... see bio for the earth
shattering ending.

k, so I checked your bio and horror of horrors ... you live in Worcester!

Maybe he didn't want to write a bunch of if statements to do the comparison directly. I don't know if "switch" in that language (looks like c#) can only compare against constants or whether they invoke the overloaded == operator.

I dunno, but just trying to possibly put logic as to why this was done. I haven't checked... But can someone check the IL that's generated when you switch a string? Is it possible that this was decompiled source? I kinda doubt it.... but I kinda want to think that there's not a person alive that is retarded enough to do command line arguments this way.... I dunno... just a wish... I don't have time to verify... If someone does check though, do both C# and VB.NET as they are known to produce different IL output for some things...

Anonymous:

No this would have been the slightly ingenious version that might not have gotten you fired (although you should still be refactored at the earliest possible chance):

 

   int hash = getHashValue(args[0].toCharArray());

    switch(hash)
    {
        case getHashValue("help".toCharArray()):
            ...
            break;
        case getHashValue("single".toCharArray()):
            ...
            break;
        case getHashValue("multi".toCharArray()):
            ...
            break;
        // etc
    }

 

It would not have worked. java compiler cannot compute the values at compile time. Note that the code is indeed correct only because Java specification defines exactly strings hash, which it does not do for other system classes. Relying on such a tiny point in the spec makes me feel unsafe...

 

Anonymous:

No this would have been the slightly ingenious version that might not have gotten you fired (although you should still be refactored at the earliest possible chance):

 

   int hash = getHashValue(args[0].toCharArray());

    switch(hash)
    {
        case getHashValue("help".toCharArray()):
            ...
            break;
        case getHashValue("single".toCharArray()):
            ...
            break;
        case getHashValue("multi".toCharArray()):
            ...
            break;
        // etc
    }

 

That doesn't look so bad, but it's not valid Java code.  All case labels in Java must be compile time integer constants.

Anonymous:

No this would have been the slightly ingenious version that might not have gotten you fired (although you should still be refactored at the earliest possible chance):

 

   int hash = getHashValue(args[0].toCharArray());

    switch(hash)
    {
        case getHashValue("help".toCharArray()):
            ...
            break;
        case getHashValue("single".toCharArray()):
            ...
            break;
        case getHashValue("multi".toCharArray()):
            ...
            break;
        // etc
    }

 

 Sure, if you don't mind generating compile errors because you didn't use a compile time constant in your case statement...

What about

 public class Test {
    public enum Argument{HELP,SINGLE,MULTI}

    public static void main(String[] args) {
        try {
            Argument firstArgument = Argument.valueOf(args[0].toUpperCase());
            switch(firstArgument) {
            case HELP:
                help();
                break;
            case SINGLE:
                single();
                break;
            case MULTI:
                multi();
                break;   
            default:
                throw new IllegalStateException("Should never happen");
            }            
        } catch (IllegalArgumentException iae) {
            usage();
            return;
        }
    }

}

 ?
 

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.
 

The IL does contain the original string. 

Perhaps, just perhaps, this was not a WTF at all, and was done intentionally in order for the compiled IL not to contain recognizable strings? Though why you would want to obfuscate the range of potential command line arguments is a bit strange...

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 {
System.out.println( args[i] );
}
}

Anonymous:

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

Still haven't (and won't) compile it, but I left out a "public" didn't I?

 

Anonymous:

I'm not sure I'm understanding this as I don't sepak Java, so could someone please expalin?

Is the WTF the nightmare case values, or does the hashing code simply not work? 

 

CAPTCHA - java, lol 

The WTF is a couple things, all related to the fact that he used hashes.

The main thing is that hashes are best treated as opaque values.  Hash values should be treated as deterministic, and hopefully a failry even distribution.  you should not count on any values, since the algorithm can change.  Maybe the hash of "help" is 972346 in this iteration, maybe it's 12345678 in the next.

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.

It's also stupid to perform this optimization.  By definition, this is done once, at program start time.  To optimize like this, even if it did work, by making something very unclear and error prone just for program startup is kind of silly.

 

At first, i didn't buy that this was real code, but i think a lot of programmers have been bit by the "hashes are fast" bit.  I remember one guy we had in our firm who heard that, and everything was a hash.  We looked at his code, and he never declared any object,s just hashes.  "well, i want to look up a value, and hashes are fast", not faster than an object with a fixed offset, nevermind the other stupidities with this. 

Anonymous:

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 think the programmer probablly heard somewhere that "Hashes are fast" and wanted to optimize what was obviously the application's bottleneck.

 

More likely his original code was:
 

if(args[0].equals('help')) {

} else if(args[0].equals("single")) {

...

 

And then he read on the daily WTF that these "if..then" trees are ugly so he converted the system to a nice, clean switch statement (which, case you are curious, doesn't work on strings in Java). 

dllexport:

Maybe he didn't want to write a bunch of if statements to do the comparison directly. I don't know if "switch" in that language (looks like c#) can only compare against constants or whether they invoke the overloaded == operator.

 

Then maybe he should've used a GetOpt package to parse his command line. The code's already written, so what's the problem? 

Since we actually appear to be trying to improve the code, why not just do it the correct way?

public static void  main (String args[])
{

    if (args.length < 1 || "help".equals(args[0])) {
       ...
    } else if ("single".equals(args[0])) {
       ...
    } else if ("multi".equals(args[0])) {
       ...
    // etc.
   }
}

 

If you feel like making the code not case sensitive you could even use equalsIgnoreCase.

On the other hand, if you just can't stand the idea of using if/else if logic you could create some nightmare code using a Map of Strings to ints used as constants and then do a switch statement using the constant.

At least by pre-calculating the hashes the author knows that there won't be any collisions...

If we want to stick to the "good" idea behind the code (allowing a switch statement on strings in Java), I think this is the best way to do it:

private static int matchString(String s, String... array) {

  for (int a = 0; a < array.length; a++)

    if (s.equals(array[s]))

      return a+1;

  return 0; //no match

}

public static void  main (String args[]) {

    switch(matchString(args[0], "help", "single", "multi")) {

        case 1: //help

            ...

            break;

        case 2: //single

            ...

            break;

        case 3: //multi

            ...

            break;

    }

}

But I still think an if-elseif-elseif-... structure looks better than that.

Anonymous:

Anonymous:

I'm not sure I'm understanding this as I don't sepak Java, so could someone please expalin?

Is the WTF the nightmare case values, or does the hashing code simply not work? 

 

CAPTCHA - java, lol 

The WTF is a couple things, all related to the fact that he used hashes.

The main thing is that hashes are best treated as opaque values.  Hash values should be treated as deterministic, and hopefully a failry even distribution.  you should not count on any values, since the algorithm can change.  Maybe the hash of "help" is 972346 in this iteration, maybe it's 12345678 in the next.

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.

It's also stupid to perform this optimization.  By definition, this is done once, at program start time.  To optimize like this, even if it did work, by making something very unclear and error prone just for program startup is kind of silly.

 

At first, i didn't buy that this was real code, but i think a lot of programmers have been bit by the "hashes are fast" bit.  I remember one guy we had in our firm who heard that, and everything was a hash.  We looked at his code, and he never declared any object,s just hashes.  "well, i want to look up a value, and hashes are fast", not faster than an object with a fixed offset, nevermind the other stupidities with this. 

 

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). 

And even though it has been said a dozen times here already (possibly even once by me) I'll say it one more time, replacing the hash values with String literals wouldn't compile.  Most likely this wasn't a half assed attempt at optimization, just the guy really, really wanted to use a switch statement instead of a bunch of else if's. 

Of course, a secondary WTF is that Java still doesn't have an interface to the standard getopt or getoptlong, nor does it have an alternative in the standard library.

Just one of the many things that are unnecessarily complicated in Java.

Newlines look better, too.

Anonymous:

Since we actually appear to be trying to improve the code, why not just do it the correct way?

public static void  main (String args[])
{

    if (args.length < 1 || "help".equals(args[0])) {
       ...
    } else if ("single".equals(args[0])) {
       ...
    } else if ("multi".equals(args[0])) {
       ...
    // etc.
   }
}

This is what I'd do. I don't think the programmer had "hashes are fast" on his mind. More of the "can't use the switch statement with Strings" line of thinking. 

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.

 

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)

{

 switch(x)

{

 case "help": Console.WriteLine("Yes you are right Java does suck");

}

}

c:>Runme

Missing arguments!

- You need to enter a string that hashes to 972346 to display this schreen or start me without arguments
- A string that hashes to -91058 causes me to do... well the string is self explaining
- Another of my options hases to -4830672 and allows you to do something really usefull! Try it out!

C:>

Sir, an ingenious solution is not "genius" (nor is it "genious", which isn't a word).

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.

 And your code doesn't work--looping through args is not the point at all. The fact that you don't understand this is the reason you are a C# programmer.
 

 

Here is the proper way to do it:

public static void main (String args[])

{

  // Get the first argument

  String arg1 = args[0];

  System.out.println("Operator, please read the following line for me:");

  System.out.println(arg1);

  System.out.println("If it says help, please type 1 and press Enter.");

  System.out.println("If it says single, please type 2 and press Enter.");

  System.out.println("If it says multi, please type 3 and press Enter.");

  InputStreamReader consoleIn = new InputStreamReader(System.in);

  int command = (consoleIn.read());

  consoleIn.close();

  switch(command)

  {

     case '1': // help

        ...

        break;

      case '2': // single

        ...

        break;

      case '3':  // multi

        ...

        break;

      default:

        System.out.println("I'm, sorry, I don't understand.  Please try again.");

        main(args);   // gotos are bad.  Recursion is good.

    }

}

 

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?

newfweiler:

  System.out.println("Operator, please read the following line for me:");

This is one of the best ideas EVAR!

But as far as an actual solution goes, couldn't you use the String as an index into a HashMap that maps to a function call? Where each function does what the individual case statements would do? This way, if you have 50,000,000 Strings to compare to, you don't have to run down the list of 50,000,000 if-else statements, execution is effectively in constant time (even though you have to load up a 50,000,000-size HashMap and write 50,000,000 subroutine functions...)

Anonymous:

At least by pre-calculating the hashes the author knows that there won't be any collisions...

Sure those values don't collide with each other.  But what about all the other strings that I could enter on the command-line?  One of them may collide...

 

-ds 

I suppose the WTF is that he/she didn't do a binary search on the hash values? ;-)
 

I'm just curious how he discovered that parsing CLA's was the bottleneck.

That's normally a task I perform once per program.

 

Captcha : Null 

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?

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?

wow, you are some kind of serious software developer!!!

it's a shame you're not a software engineer or you'd know that in many cases (those in which it's more efficient than a sequence of String.Equals() compares) c# actually handles the switch exactly the same way as the code in the initial post - hashing the string and doing a hash compare.

good try, though.  c- for effort. 

Anonymous:

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

 

 

 

What!?? are you f'n nuts?!!

 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.

I think I've seen this code before.

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?

 

Who cares? use GetOpt