Is it a Number?



  • Hello,
    This one didn't make it to the front page, but I thought I should still share it with everybody here in the sidebar.
    We are developing this GUI application, and in this application we have several text fields that should accept only positive numbers. We noticed that for some reason these fields wouldn't let you type the digit '9' in. We looked into the bug, and found the following piece of code:
     
       public static boolean isNumber(char c){
    boolean flag = false;
    for(int i=0;i<9;i++){
    if(c == "0123456789".toCharArray()[i]){
    flag = true;
    }
    }
    return flag;
    }
     
    I'll let you appreciate this wonderful piece of WTFery, that becomes more and more scarce on the main page... 


  • Ok, what's with this forum software and line wrapping?



  • Brillant stuff!

    I love the way this function manages to use a thoroughly ridiculous algorithm to do something simple, implement the algorithm as inefficiently as possible (what's with this flag = true thing, instead of just returning true?? it has to take the exact same number of cycles for every possible digit?) and also introduce a very silly bug in the process. It's one of the best contributions in a long time in all its simplicity.



  • This is the shortest instance of for-case I've ever seen.

    However, it isn't THAT bad... to implement this, comparing against all characters in "0123456789" is a possible way to go. But not that way... and not converting it to a char array each time...

    In C I would have done it like

    return !!strchr("0123456789", c);

    and I bet a similar one-liner is possible in Java. If it should be efficient, I'd rather use a switch statement. But... the real WTF probably is that there is already a built-in isdigit() somewhere...



  • Very nice and simple WTF. To that isDigit() thingee...

     

     

    http://java.sun.com/j2se/1.5.0/docs/api/java/lang/Character.html#isDigit(char) 

    http://java.sun.com/j2se/1.5.0/docs/api/java/lang/Character.html#digit(char,%20int)

     

    However I dont understand what a radix is, so I cant tell which to use. 



  • Radix is the number-system base to use. ie, 10 for decimal, 2 for binary, 16 for hexadecimal, etc.



  • [quote user="Hans"]

    what's with this flag = true thing, instead of just returning true??

    [/quote]

    Single exit point, you know. Even though the whole function is buggy and idiotic, it should at least embrace what some people consider "good style".



  • public static boolean isNumber(char c)

    {
        return c >= '0' && c <= '9';

    }



  • [quote user="ammoQ"][quote user="Hans"]

    what's with this flag = true thing, instead of just returning true??

    [/quote]

    Single exit point, you know. Even though the whole function is buggy and idiotic, it should at least embrace what some people consider "good style".

    [/quote]

     In that case, a "break" would have been nice :)



  • [quote user="OpBaI"] I bet a similar one-liner is possible in Java. [/quote]

    Something like 'return "0123456789".indexOf(c) > -1;' would do it, if you really wanted to do it that way. 



  • Nice WTF :)



  • [quote user="Cloaked User"]

    [quote user="OpBaI"] I bet a similar one-liner is possible in Java. [/quote]

    Something like 'return "0123456789".indexOf(c) > -1;' would do it, if you really wanted to do it that way. 

    [/quote]hehe or how about regexp: return ((String)(c+"")).matches("[0-9]");

     



  • Good one.  Maybe we should have an additional "sidebar" forum for code WTF's that don't make the glamour cut.



  • [quote user="nbit"]

    public static boolean isNumber(char c)

    {
        return c >= '0' && c <= '9';

    }

    [/quote]

    Should be isDigit(char c)....



  • [quote user="savar"][quote user="nbit"]

    public static boolean isNumber(char c)

    {
        return c >= '0' && c <= '9';

    }

    [/quote]

    Should be isDigit(char c)....

    [/quote]

    No... You don't want to call it isDigit() because there is already an isDigit function.   hmmm.  Hey wait a minute guys!  I think I have an idea!!! 



  • [quote user="OpBaI"]In C I would have done it like return !!strchr("0123456789", c); and I bet a similar one-liner is possible in Java. If it should be efficient, I'd rather use a switch statement. But... the real WTF probably is that there is already a built-in isdigit() somewhere...[/quote]

    As others have pointed out, there is a Character.isDigit(char) method.

    Of course, it's a WTF that the isNumber method was written at all.  How does one manage to write Java without knowing about Integer.parseInt(String)?

    For that matter, a professional application would parse user input using NumberFormat.getIntegerInstance().parse(String), since it gives you internationalization and lenient parsing for free.



  • [quote user="marvin_rabbit"][quote user="savar"][quote user="nbit"]

    public static boolean isNumber(char c)

    {
        return c >= '0' && c <= '9';

    }

    [/quote]

    Should be isDigit(char c)....

    [/quote]

    No... You don't want to call it isDigit() because there is already an isDigit function.   hmmm.  Hey wait a minute guys!  I think I have an idea!!! 

    [/quote]


    The really sad thing is that you still haven't picked up the underlying problem with this code. Your makeing the assumption that it is encoded as ASCII. What about all the other really fun codecs who don't have a nice unbroken binary sequence between 0 and 9.

    Not that i can think of any, but as IT pro's we should at least entertain that idea.


  • I work with a "programmer" who has garbage like this everywhere.  I may have even posted one just like it, only to not have it show up on the front page.  Another gem I found was, rather than just making sure to set a pointer to NULL when he deleted it, he would "call any old function" (actual comment) on the pointer in a try/catch block to see if it threw an exception.



  • [quote user="Hans"]

    Brillant stuff!

    I love the way this function manages to use a thoroughly ridiculous algorithm to do something simple, implement the algorithm as inefficiently as possible (what's with this flag = true thing, instead of just returning true?? it has to take the exact same number of cycles for every possible digit?) and also introduce a very silly bug in the process. It's one of the best contributions in a long time in all its simplicity.

    [/quote]

    Perhaps ported straight over from visual basic, where that is pretty much just how functions have to be written. Do any other languages enforce single-exit-point? (without say, the use of gotos)



  • @Azrael said:

    [quote user="marvin_rabbit"][quote user="savar"][quote user="nbit"]

    public static boolean isNumber(char c)

    {
        return c >= '0' && c <= '9';

    }

    Should be isDigit(char c)....

    [/quote]

    No... You don't want to call it isDigit() because there is already an isDigit function.   hmmm.  Hey wait a minute guys!  I think I have an idea!!! 

    [/quote]


    The really sad thing is that you still haven't picked up the underlying problem with this code. Your makeing the assumption that it is encoded as ASCII. What about all the other really fun codecs who don't have a nice unbroken binary sequence between 0 and 9.

    Not that i can think of any, but as IT pro's we should at least entertain that idea.
    [/quote]

    Well, we should really consider making some Enterprise functionality here, an IsNumberTesterProvider, and of course an IsNumberTesterProviderFactory class to handle creating new IsNumberTesterProviders (taking a LocalizedNumberTesterFormatProviderFactory object as an argument, of courser).



  • [quote user="LordOfThePigs"]

       public static boolean isNumber(char c){
    boolean flag = false;
    for(int i=0;i<9;i++){
    if(c == "0123456789".toCharArray()[i]){
    flag = true;
    }
    }
    return flag;
    }

    [/quote]

    So let me get this straight...not only does it not work, but the doesn't work part doesn't work, either?  Awesome.



  • [quote user="Azrael"][quote user="marvin_rabbit"][quote user="savar"][quote user="nbit"]

    public static boolean isNumber(char c)

    {
        return c >= '0' && c <= '9';

    }

    [/quote]

    Should be isDigit(char c)....

    [/quote]

    No... You don't want to call it isDigit() because there is already an isDigit function.   hmmm.  Hey wait a minute guys!  I think I have an idea!!! 

    [/quote]


    The really sad thing is that you still haven't picked up the underlying problem with this code. Your makeing the assumption that it is encoded as ASCII. What about all the other really fun codecs who don't have a nice unbroken binary sequence between 0 and 9.

    Not that i can think of any, but as IT pro's we should at least entertain that idea.
    [/quote]

    Having the digits be in consecutive increasing code points is the one thing you can count on: even EBCDIC did it this way.


  • Even if you don't want to use 'break' or 'goto' or an extra exit point, there really is no excuse for not doing at the very least this:

               for (int i=0; !flag && (i<=9); i++)
    

    By the way, there is something disturbing about using a flag variable called "flag", I think. Using a generic loop index called 'i' is often acceptable since it's so well established, but this is just wrong.

    (And yes, I did fix the loop condition. Couldn't stand it.)



  • A quick check of our own codebase turns up something similar:

    private boolean startWithDigit( String test_string )
    {
    	boolean result = false;
    
    String digits = &quot;0123456789&quot;;
    if ( digits.indexOf( test_string.substring(0,1)  ) &gt; -1 )
    {
    	result = true;
    }
    <br />
    return result;
    

    }

    Clearly, this entire thing could have been replaced with:

    Character.isDigit( test_string.charAt( 0 ) )


  • [quote user="Hans"]

    Brillant stuff!

    I love the way this function manages to use a thoroughly ridiculous algorithm to do something simple, implement the algorithm as inefficiently as possible (what's with this flag = true thing, instead of just returning true?? it has to take the exact same number of cycles for every possible digit?) and also introduce a very silly bug in the process. It's one of the best contributions in a long time in all its simplicity.

    [/quote]

    Maybe the flag = true instead of returning true is a code convention. Methos should have only one and only one exit point... avoid returning from multiple places is a good habit to make the code easy to mantain later.


Log in to reply