Comment On It Never Hurts To Ask

weird  

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?

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

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

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

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

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.

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

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? :)

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

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.

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

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.

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!

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.

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(]

Oh.



My.



GOD.

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.

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.

 

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

Nice.



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



Nice.

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



//what meaning would wtf have?

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

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!

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.

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.

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:

import re
def isAlphaNumericOnly(value):
   return re.match( '^[-\w#\. ]+$', value )

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

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

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.

class TPSReport {
   function TPSReport (boolean exists) 
    { this.exists = exists }

   function DoesThisTPSReportExist()
    { if (!this.exists) 
         { throw new MetaphysicalException("sorry, I don't exist") }
      return this.exists ;
    }
}

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!

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.

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.

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

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

John Smallberries:

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

 

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

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!

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.

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

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.

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

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!

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

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

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

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.

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

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.

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.

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.