• javaman (unregistered)

    weird  

  • (cs)
    Alex Papadimoulis:

        for (int i = 0; i < character.length; i++) {

          isAlphaNumeric = (Character.isLetter(character[i])

                           || Character.isDigit(character[i])

                           || Character.isSpaceChar(character[i])

                           || character[i] == '.' 
                           || character[i] ==     '#'
                           || character[i] ==     '-');

     

     

      Correct me if I'm wrong, but doesn't this loop end up setting the value of isAlphaNumeric based on only the last character in the array?

  • eddie (unregistered) in reply to limelight
    limelight:

      Correct me if I'm wrong, but doesn't this loop end up setting the value of isAlphaNumeric based on only the last character in the array?

    Nope. 'Cause it throws an 'illegalArgumentExeption'...
  • (cs) in reply to limelight

    I think it will stop and throw the exeception if it does find one.  I am not sure though.

  • eddie (unregistered) in reply to eddie

    note to self: do not edit an quote...

  • mol (unregistered) in reply to limelight

    no, because it throws exception on the first one !isAlphaNumeric character

  • Random Internets Guy (unregistered) in reply to limelight

    The conditional that appears in the original right after your excerpt is tested on each character sequentially; so if it encounters any character that's not alphanumeric, it will throw the exception.

    Rather than, you know, returning false forthwith.

  • GeneralPF (unregistered) in reply to limelight

    No, because at any point in the loop, if isAlphaNumeric gets set to false, it will throw an exception and leave.

  • Greg Barton (unregistered) in reply to limelight

    Nah.  The test right after the assigment will throw an exception as soon as a bad character is encountered.  But the funny thing is the method will never return false. (An exception will be thrown first.)

    Well, I take that back.  It'll return false if isAlphaNumeric is initialized to false and a zero length string is input.

    WTF? :)

  • Turbo (unregistered) in reply to limelight

    No, because if it finds a non-alphanumeric character it throws an exception.

  • (cs) in reply to limelight
    limelight:
    Correct me if I'm wrong, but doesn't this loop end up setting the value of isAlphaNumeric based on only the last character in the array?


    Hey, in case no one has said it(I don't read comments, teehee!), it'll throw an exception if it finds a non-alphanumeric character.
  • Rich C. (unregistered)

    Looks like a good opportunity to use a regexp...  All that looping, what a waste...


  • nobody (unregistered) in reply to Turbo
    Anonymous:
    No, because if it finds a non-alphanumeric character it throws an exception.
    No, because the zero-length empty string will cause it to never enter the loop.
  • Genny (unregistered)

    This hurts!

    Someone has read 3 pages of "OOP for noobies" and started to write a class for each dumb validation? I really love his constructor. It's great to take field values from outside, you can be sure they all get initialized. It's even better to use fields instead of local method variables.

    I want this guy for my project!

  • (cs)

    What's the point of passing in isAlphaNumeric in the constructor?
    Looks like it could be used to notify the instance that it should accept only alpha, but it's simply overwritten during test.

  • (cs) in reply to nobody

    Now all we need is code that goes:

    if (!myFormat.isAlphaNumericOnly(inputValue)) { // execute and untested method which never works}

    Then when the untested method blows up, people will realize that isAlphaNumericOnly returned false and then come to the wrong conclusion that bad characters were entered when in fact no characters belonged to the string. Brillant! [+o(]

  • (cs)

    Oh.

    My.

    GOD.

  • php programmer (unregistered)

    That is funny, it sounds like the person didn't have a final plan worked out, started writing a boolean data testing function, then decided to add exceptions part way through (or retro). 

    What would be amusing is to see if somewhere else the function is called and see a try {fieldFormat.isAlphaNumericOnly(text)} catch(...) {treatAlphaFalse...} type statement, since you can't use it to check what something is without throwing an exception.

    Maybe he's used to working with quantum states, where its normal that when you can't just check something without changing the direction and flow of the experiment.

  • (cs)

    Haha, another horrible abuse of exceptions.  I never get sick of these.  Imagine the code to call this:

    boolean goodInput = false;

    try

    {

    goodInput = field.isAlphaNumberic(myString)

    }catch(IllegalArgumentException e)

    {
    }

     

    That's some good stuff right there.

     

  • (cs) in reply to OneFactor

    Forgot to add. Wow! Thanks for making my day Alex. This is the best in a very long time.

  • (cs) in reply to javaman

    Nice.

    Call a validation method that goes BOOM if you don't pass in valid data.

    Nice.

  • (cs) in reply to christoofar

    bool wtf = new FieldFormat(false).isAlphaNumericOnly("");

    //what meaning would wtf have?

  • (cs)

    Classic, a validiation routine to avoid exceptions, that throws an exception. Brillant!

  • (cs) in reply to christoofar
    christoofar:
    bool wtf = new FieldFormat(false).isAlphaNumericOnly("");

    //what meaning would wtf have?


    oohh ohh!  I know!  It would be false!

    I am so smart
    S-M-R-T!
  • (cs) in reply to limelight
    limelight:
    Alex Papadimoulis:

        for (int i = 0; i < character.length; i++) {

          isAlphaNumeric = (Character.isLetter(character[i])

                           || Character.isDigit(character[i])

                           || Character.isSpaceChar(character[i])

                           || character[i] == '.' 
                           || character[i] ==     '#'
                           || character[i] ==     '-');

     

     

      Correct me if I'm wrong, but doesn't this loop end up setting the value of isAlphaNumeric based on only the last character in the array?



    Yes.  That's not really a WTF, because if it fails the conditional statement---BOOM.   Also this method ALWAYS returns true... of course unless you called the ctor() with a value of false and hit this method with an empty string.

    TOTALLY useless class.  Should be chucked in the bin.
  • (cs) in reply to Rich C.
    Anonymous:
    Looks like a good opportunity to use a regexp...  All that looping, what a waste...




    Actually looping through a string's characters is generally less expensive than regex.  So it's not really much of a waste at all.
  • diaphanein (unregistered) in reply to zip

    I happen to love the use of a member variable for what ought to have been a local.  I also love how symbols and whitespace is considering alphanumeric.  Last I heard, alphanumber is general characters and digits.

    Just for giggles, here's the validator all tidy like:

    <FONT face="Courier New" size=2>import re
    def isAlphaNumericOnly(value):
       return re.match( '^[-\w#\. ]+$', value )</FONT>

    I still object to whitespace and punctuation being alphanumeric...

  • (cs)

    Wow, this is the most terrible Java code I've ever seen.  At least the Brillant Paula Bean didn't really try to do anything..

    This code has to be legit, because you can't make up stuff like this.

    Makes my brain hurt....

  • (cs)

    Nice.  So let's see just how many WTFs there are in this one class:

    1. private variable not used (written, but not read)
    2. constructor taking meaningless argument
    3. boolean-valued method that cannot return "false"
    4. incorrect definition of "alphanumeric" (normally does not include punctuation)

    It may be that snipped parts of the class make use of the otherwise useless private variable, but that would be a WTF too, since it still doesn't look like the flag is associated with other class-scope data as would normally be expected, but only calculated based on transient arguments to class methods.  (In this case the variable should be called "wasLastArgumentTo_IsAlphaNumericOnly_AlphaNumeric", which may or may not make any sense for this application...)

    Hmm, not quite a 1:1 line-to-WTF ratio, but a good try nonetheless.

  • greensweater (unregistered)

    <FONT face="Courier New">class TPSReport {
       function TPSReport (boolean exists) 
        { this.exists = exists }</FONT>

    <FONT face="Courier New">   function DoesThisTPSReportExist()
        { if (!this.exists) 
             { throw new MetaphysicalException("sorry, I don't exist") }
          return this.exists ;
        }
    }</FONT>

  • (cs) in reply to Ytram
    Ytram:
    Anonymous:
    Looks like a good opportunity to use a regexp...  All that looping, what a waste...




    Actually looping through a string's characters is generally less expensive than regex.  So it's not really much of a waste at all.

    Hey Oliver!
    Didja see this? Flame on!
  • (cs) in reply to John Smallberries
    John Boysenberries:
    Hey Oliver!
    Didja see this? Flame on!


    I... don't get it.  Should I be scared?  Will there be an actual fire, or is "flame" being used as a euphemism of some sort?

    I would like to qualify my statement as pertaining only to looping through strings versus regex verification, and not to today's WTF as a whole.  I would also like to further state that early optimization is bad.
  • (cs) in reply to Ytram
    Ytram:
    John Boysenberries:
    Hey Oliver!
    Didja see this? Flame on!


    I... don't get it.  Should I be scared?  Will there be an actual fire, or is "flame" being used as a euphemism of some sort?

    I would like to qualify my statement as pertaining only to looping through strings versus regex verification, and not to today's WTF as a whole.  I would also like to further state that early optimization is bad.

    Oliver and I were just having a discussion on this very topic (parsing/looping vs. regex), and I thought I would goad him into action. The gist is that regex usually simplifies the code so much that the potential performance penalty is well worth it.

    Flame as in...well...flame.
  • (cs)

    The question is whether isAlphanumeric is used in other places in this class.  That would be really awesome.

  • (cs) in reply to John Smallberries
    John Smallberries:
    The gist is that regex usually simplifies the code so much that the potential performance penalty is well worth it.


    Because regular expressions are known for being so easy to read and understand.... :)
  • (cs) in reply to John Smallberries

    John Smallberries:

    Flame as in...well...flame.

     

    I hope you don't mean definition 4...

  • (cs)

    I devised a brillant function that makes this method easier to use - the DoesThrow function:

    public boolean DoesThrow(Runnable what)
    {
        try
        {
           what.run();
        }
        catch (Exception ex)
        {
           return true;
        }
        throw new IllegalArgumentException("Error: Does not throw exception");
    }

    Now a simple if(IsTrue(DoesThrow(new Runnable() { public void run() { fieldFormat.IsAlphaNumericOnly(myString); } }))) check can be used to test if myString is not alpha-numeric only. And the most ingenuios part is that you can also check for the converse by simply using IsTrue(DoesThrow(DoesThrow(...))) instead!

  • (cs) in reply to kipthegreat
    kipthegreat:
    John Smallberries:
    The gist is that regex usually simplifies the code so much that the potential performance penalty is well worth it.


    Because regular expressions are known for being so easy to read and understand.... :)


    Come to think of it, I'm surprised that I haven't seen any regular expression WTF's since I've been reading this site..  Maybe they're just too brilliant for brillant programmers to even use incorrectly.
  • (cs)

    And of all things it throws an instance of RuntimeException.  For all you Java haters out there, this is atypical use of exceptions.

  • (cs) in reply to kipthegreat
    kipthegreat:
    John Smallberries:
    The gist is that regex usually simplifies the code so much that the potential performance penalty is well worth it.


    Because regular expressions are known for being so easy to read and understand.... :)

    In fact, yes.
    The search expressions themselves can be rather complex, but rarely is it hard to understand what's happening.
  • (cs) in reply to John Smallberries
    John Smallberries:

    Oliver and I were just having a discussion on this very topic (parsing/looping vs. regex), and I thought I would goad him into action. The gist is that regex usually simplifies the code so much that the potential performance penalty is well worth it.

    Flame as in...well...flame.


    In .NET, you can also compile frequently-used regexes to bytecode. A smart regex engine could in fact generate code that is as fast as or faster than hand-written C# code (particularily if it is hand-written by a moron).
  • Nand (unregistered)

    Yuck! Let's see:

    1) Pointless variable isAlphaNumeric. If it's not set to true, the entire class is unneccesary.
    2) Equally pointless constructor, because...
    3) The isAlphaNumeric function should be STATIC
    4) Using a class variable that's supposed to have been initialized to a specific value as a temp variable in a loop
    5) Accepting non-alphabetical, non-numerical values as alphanumeric
    6) Throwing an exception instead of returning false
    7) Function never returns false
    8) looping through each character of a string instead of using String.matches("regex");

    yay!

  • (cs) in reply to Nand
    Anonymous:
    Yuck! Let's see:

    1) Pointless variable isAlphaNumeric. If it's not set to true, the entire class is unneccesary.
    2) Equally pointless constructor, because...
    3) The isAlphaNumeric function should be STATIC

    Duh, the isAlphaNumeric instance variable is very useful, it tells you what you'll get out of your verification function when the patter IS alphanumeric (cause, you know, if it isn't you won't get nuttin)

    Oh, and the "isAlphaNumeric" variable should not exist, not as part of the instance nor as part of the class at least.

    7) Function never returns false

    Yes it does, try

    boolean result = new FieldFormat(false).isAlphaNumeric("")

    Voilà, function returns false

  • (cs) in reply to Nand
    Anonymous:

    8) looping through each character of a string instead of using String.matches("regex");


    I'm pretty sure this is Java code.  If so, regular expressions are a pretty recent (version 5) addition to the language... so they probably were not an option.... not that he/she would have used them if they were there...
  • (cs)

    This code is so half-assed. The developer comes up with a brilliant alphanumeric validator but uses tired, old character methods. He should use his own home-grown isLetter, isNumber, and isSpaceChar methods, each having the same algorithm as isAlphanumeric. That way, pretty much every reasonable input will throw a exception. Why is that good? Because if exceptions happen nearly all the time, then, logically, they're not exceptions anymore! Error-proof code!

    --Rank

  • (cs) in reply to Nand
    Anonymous:
    8) looping through each character of a string instead of using String.matches("regex");


    I'm not going to say one way is better than another, but looping through the characters of a string is not a WTF.  It's something that should be up to the coder or company coding standards.  As I mentioned earlier, looping through the characters is generally less expensive than regex.  As for readability and maintainability, that's where it's subjective and up to the coder.
  • (cs) in reply to kipthegreat

    >I'm pretty sure this is Java code. 
    >If so, regular expressions are a pretty recent (version 5) addition to the language

    Actually java.util.regex package was added with Java1.4.  (ie the version before version 5)
    That still doesn't excuse the programmer. 
    My guess is that originally instead of throwing the exception, it set the variable to false.
    Then they either changed their mind, or someone came along afterwards to change the code.
    Either way it is still a very good example of WTF?

  • (cs) in reply to Nand
    Anonymous:
    Yuck! Let's see:

    1) Pointless variable isAlphaNumeric. If it's not set to true, the entire class is unneccesary.
    2) Equally pointless constructor, because...
    3) The isAlphaNumeric function should be STATIC
    4) Using a class variable that's supposed to have been initializzzzzzed to a specific value as a temp variable in a loop
    5) Accepting non-alphabetical, non-numerical values as alphanumeric
    6) Throwing an exception instead of returning false
    7) Function never returns false
    8) looping through each character of a string instead of using String.matches("regex");

    yay!

    You can You can add "9) Javadoc not used to document class."  The uselessness of this class would become appparent if this were done.
    add "9) Javadoc not used to document class."  The uselessness of this class would become appparent if this were done.

  • Frostcat (unregistered) in reply to Rich C.
    Anonymous:
    Looks like a good opportunity to use a regexp...  All that looping, what a waste...


    Regexp? Nah, this is a perfect time for a state machine.

    Written in Javascript.

  • (cs) in reply to kipthegreat

    kipthegreat:
    John Smallberries:
    The gist is that regex usually simplifies the code so much that the potential performance penalty is well worth it.


    Because regular expressions are known for being so easy to read and understand.... :)


    I find them simpler than string matching methods involving juggling indexes or using mid()...

    So, I hereby issue a challenge...

    I'll make up a random required syntax...

    var = { subvar1 = value1 subvar2 = value2 subvar3 = { i, i2, i3, i4 } } \n

    Now, who's got the most efficient validation for my syntax?
    I would be keen to see efficient, simple, non regex based solutions to this, I really would.

    But yes, there are some ugly regexes out there, so just use a regex engine that allows commenting within a regex pattern.

     

Leave a comment on “It Never Hurts To Ask”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article