• (cs)

    <font size="2">omg!
    reenter should not be hyphenated.</font>

  • (cs)

    These are alphanumeric characters?

        _ @ - .

  • (cs) in reply to Brendan Kidwell
    Brendan Kidwell:
    These are alphanumeric characters?

        _ @ - .



    If thats your biggest concern, I envy you
  • Darrin (unregistered)

    OK, I'm guessing that JavaScript already has some sort of isAlpha or isNumber function or functions that could have been used in the first place, right? (Of course, they probably don't count @,_,-, or . as alphanumeric.)

    What is TRIM2 and how does it differ from plain old TRIM? And why doesn't its name indicate how it's different?

    Wouldn't it have been easier to do the comparison as a range or set of ranges. Like if (n <= 'a') && (n <= 'z'), etc.? I mean, even if you have to account for EBCDIC, you can still use sets of ranges.

    Why is the developer setting the variable to all uppercase after he's tested it against both upper and lower case letters? And why does he do it each time through the loop?

    Um, does this function actually return anything? It doesn't look like it returns true or false, but I'm not a JavaScript programmer so maybe I'm missing it.

    Also, I really like the alert message. "Only Alphanumeric are allowed." Alphanumeric what? And why did they capitalize "Alphanumeric?" Plus, it sure was courteous of them to erase the value the user entered so that they'll have to enter the entire string again.

    And people wonder why, as a developer, I hate most software.

  • (cs)

    Ok...

    1) Allowing non-alphanumeric characters in a routine called "IsAlphaNumeric"

    2) Testing each character separately instead of the entire word with something like Regex

    3) Testing each character with a huge, 66 part AND statement instead of something like comparing ASCII value ranges

    4) displaying an alert for EACH INVALID CHARACTER IN THE STRING!!

    5) Changing it to uppercase only AFTER the comparisons. Moving that before would save you, um, at least 26 comparisons

    6) Is "&&" a short-curcuiting operator in JavaScript? If not, the letter 'a' would still compare 65 more times.

    hehe. I'd be curious to see his TRIM2() function. Might have some equally tasty tidbits in there.... :)

  • (cs)

    Holy smokes.

    1)No regular expression, let's just check every character.

    2)We'll force the textbox's value to uppercase. Every time we check a character.

    3)We'll alert the user EVERY TIME we encounter an unacceptable character, even if there are 12 of them in the same text box. BREAK statements are for suckers, and users need to be taught a lesson.

    4)Explicitly return from the function when the control flow can't do anything BUT.

  • Simon (unregistered)

    1 - hungarian 2 - hardcoded character values 3 - _@-. are not alphanumeric 4 - empty field will be treated as alphanumeric 5 - all-space field will be treated as alphanumeric 6 - name suggests will return boolean, instead opens an alert on failure. 7 - return where none needed 8 - name suggests tests field value, instead it modifies it 9 - setting value to value uppercase inside loop 10 - unnecessary checks for lowercase characters (could move the uppercase to the start) 11 - opening window alert inside the loop

    Hell. I give up. there's approximately 1 error per 0.5 characters in this. It's like shooting fish in a barrel.

    Simon

  • Kurt (unregistered) in reply to John Bigboote

    John Bigboote:
    Holy smokes.
    3)We'll alert the user EVERY TIME we encounter an unacceptable character, even if there are 12 of them in the same text box. BREAK statements are for suckers, and users need to be taught a lesson.

    I believe that once the field is set to blank, the loop will exit as i > cfield.value.length.

  • (cs) in reply to Darrin
    Anonymous:
    What is TRIM2 and how does it differ from plain old TRIM? And why doesn't its name indicate how it's different?


    As a TRANSACT-SQL developer might say:

    What is "TRIM"?  I only know of
    LTRIM: trims all spaces from the beginning of the string
    and
    RTRIM: trims all spaces from the end of the string

    TRIM2 must thus be a user function which trims both ends of the string (hence the 2)
  • (cs) in reply to Simon

    the alert call is no big deal, it will happen only once, becuase just after displaying the message, it helpfully clears the invalid data the user typed in.

  • (cs)

    Actually, I just realized that by setting the text box to an empty string, the loop will basically end because the loop counter will be greater than the length of the string. Thus, no multiple warnings.

    Ironic that one bad practice (not getting the textbox value into a variable) saved the developer from discovering another.

  • (cs)

    Thanks everybody for noticing my error while I was typin' and testin'. Yes, I have actually popped this shit code into a web page.

  • Brian (unregistered) in reply to Mike R

    Thereby keeping him from learning from his mistakes, and probably ending up typing the same thing again [:'(]

  • (cs) in reply to Brian

    Then there's substringing instead of charAt() to add to the list.

  • (cs) in reply to Stan Rogers

    Setting the field to blank.  "You typed one wrong letter so now I'll make you retype the whole thing."  What a pain in the ass.

  • (cs)

    I discovered the ebay auction in the comments of yesterday's WTF.

    I found the concept interesting and was able to get immediate feedback from the seller about a split shipment, sending the ones to one place and the zeros to annother.

    I blogged about it, but in the end, I use a product that has the ability to generate guids so I don't need to buy one at auction. http://weblogs.asp.net/andrewseven/archive/2005/06/23/EbayGuid.aspx

  • chep (unregistered) in reply to Stan Rogers

    I guess regex'es are for suckers only!

    The whole function could be replaced with simple check on <FONT face="Courier New" color=#000080>/[^\w@\-\.]/</FONT>

    [;)]

  • (cs)

    In the VB world, we also have LTrim() and RTrim() for truncating extra spaces on the left and right side, respectively. To take care of both simultaneously, we have a function called Trim(). There are no such built-in functions for Javascript, so this guy clearly wrote his own. Except that by calling it TRIM2(), he thereby proves to us that it's one better. Maybe it washes your car as it trims off whitespace, who knows.

  • (cs)

    Here's a couple of WTFs (or, at least, design problems) nobody noticed yet:

    1. The 'i' in the for loop. Because of the way JavaScript handles variables, that 'i' is now defined as a global (to be local to the function, it'd have to be defined with a 'var i' somewhere in the function)
    2. It's pretty easy to just set up a field to only accept AlphaNumerics (even for values of alphanumerics that includes @, -, and .) in the first place. Assign an event handler that returns false for keys outside the range you want.

    --AC
  • Special (unregistered)

    Noone seems to have covered this wtf: The whole logic is mixed in with the page implementation. This wtfery is probably copied and pasted for every form field that needs to be validated.

  • (cs) in reply to Stan Rogers
    Stan Rogers:
    Then there's substringing instead of charAt() to add to the list.

    JavaScript has an indexer for strings, so you can just use:
    n = cfield.value[i];
  • (cs)

    Let me offer an insight into how this was written:

    1. The initial loop was written. It only dealt with uppercase.

    I enter some random text that includes leading spaces.

    Oops! It gives errors 25 times.

    2. I'll fix it by trimming spaces.

    Oops... that helped but still annoying.

    3. Hmm it only handles uppercase.

    Hmm I better convert the text to uppercase.

    That works! (The test string had an initial cap...)

    4. Maybe inserting a return statement would help.

    Nope, I tried that. It didn't cause any errors, so I'll leave it there.

    5. Maybe if I blank out the text field it will stop bugging me.

    Perfect! Whoops it doesn't cope with lowercase letters at the beginning of a string...

    6. Added 26 more comparisons.

    That's how I should have done it in the first place.

    Perfect!
  • (cs) in reply to Special

    Anonymous:
    Noone seems to have covered this wtf: The whole logic is mixed in with the page implementation. This wtfery is probably copied and pasted for every form field that needs to be validated.

    He passes the field in as a parameter

  • (cs) in reply to Free

    Free:
    ...

    You might want to correct the spelling of 'speech' in your signature.  Unless you meant 's peach', then you'll probably need to explain what that means.

  • R.Livsey (unregistered) in reply to Special
    Anonymous:
    Noone seems to have covered this wtf: The whole logic is mixed in with the page implementation. This wtfery is probably copied and pasted for every form field that needs to be validated.


    The form field object looks to be passed in as  'cfield' so  can actually be used for any form field.

    Although what the 'c' stands for in the hungarianesque notation I don't know.
  • (cs) in reply to R.Livsey
    Anonymous:
    Anonymous:
    Noone seems to have covered this wtf: The whole logic is mixed in with the page implementation. This wtfery is probably copied and pasted for every form field that needs to be validated.


    The form field object looks to be passed in as  'cfield' so  can actually be used for any form field.

    Although what the 'c' stands for in the hungarianesque notation I don't know.


    Cocktostoen
  • (cs) in reply to Brendan Kidwell
    Brendan Kidwell:
    These are alphanumeric characters?

        _ @ - .

    I'm guessing the author of the code was trying to validate e-mail addresses, so the real name of the method should have been isEMailAddress() (of course he probably then decided I think I'll use this method for all the other fields on the page too...so I don't have to rewrite all those tests)...

    The answer to your question has a boatload of caveats to it.

    _ @ - . are not letters or digits.

    However, - and . can be part of a number (-1.23). So if you are looking at an individual character i.e. is this a letter or a digit the answer is definitely no. If the question is, is this character sequence a valid number? The answer is yes (unless decimals and/or negatives aren't allowed).

    In Java Character.isDigit('-') returns false (as does '.'). however, Double.parseDouble("-1.23") returns a double of value -1.23.

  • (cs) in reply to strongarm

    strongarm:
    Brendan Kidwell:
    These are alphanumeric characters?

        _ @ - .

    I'm guessing the author of the code was trying to validate e-mail addresses, so the real name of the method should have been isEMailAddress() (of course he probably then decided I think I'll use this method for all the other fields on the page too...so I don't have to rewrite all those tests)... The answer to your question has a boatload of caveats to it. _ @ - . are not letters or digits. However, - and . can be part of a number (-1.23). So if you are looking at an individual character i.e. is this a letter or a digit the answer is definitely no. If the question is, is this character sequence a valid number? The answer is yes (unless decimals and/or negatives aren't allowed). In Java Character.isDigit('-') returns false (as does '.'). however, Double.parseDouble("-1.23") returns a double of value -1.23.

    What does this have to do with whether those chars are "alphanumeric characters"?

  • Boojum (unregistered) in reply to dubwai

    Granted it's the least of the problems, but I pity anyone who might have to i18nize that someday.

  • chep (unregistered) in reply to strongarm

    strongarm:
    Brendan Kidwell:
    These are alphanumeric characters?

        _ @ - .

    I'm guessing the author of the code was trying to validate e-mail addresses, so the real name of the method should have been isEMailAddress()

    [...]

    If author was trying to validate e-mail address then [s]he is way off... [*-)]

    I mean ....@This@[email protected]_all.right...... (dots are included [:)])

    [email protected] [:(]

  • (cs) in reply to R.Livsey
    Anonymous:
    Anonymous:
    Noone seems to have covered this wtf: The whole logic is mixed in with the page implementation. This wtfery is probably copied and pasted for every form field that needs to be validated.


    The form field object looks to be passed in as  'cfield' so  can actually be used for any form field.


    I'm sure that this didn't stop our heroic chicken slayer from copying/pasting this function into every page that needed it.
  • chep (unregistered) in reply to John Smallberries

    John Smallberries:

    JavaScript has an indexer for strings, so you can just use:
    n = cfield.value[i];

    RTFM! [8-|]

    In javascript, object String does not have indexer!

    VBScript does.

  • (cs) in reply to chep
    Anonymous:

    John Smallberries:

    JavaScript has an indexer for strings, so you can just use:
    n = cfield.value[i];

    RTFM! [8-|]

    In javascript, object String does not have indexer!

    VBScript does.


    RTFM if you want to, but I replaced one line with the other and it worked fine.
        //var n = cfield.value.substr(i,1);
        n = cfield.value[i];

  • Andre (unregistered)

    I got a bonus wtf:

    He used "cfield.value.length" inside the for loop condition. Now mind you, it wouldn't work otherwise due to him setting the string to ''  possibly, but instead of checking the string length in the for loop he should have broken out of the loop instead.

  • chep (unregistered) in reply to John Smallberries
    John Smallberries:
    Anonymous:

    John Smallberries:

    JavaScript has an indexer for strings, so you can just use:
    n = cfield.value[i];

    RTFM! [8-|]

    In javascript, object String does not have indexer!

    VBScript does.


    RTFM if you want to, but I replaced one line with the other and it worked fine.
        //var n = cfield.value.substr(i,1);
        n = cfield.value[i];

    Yeah! n is always undefined now...  [:)]

  • chep (unregistered) in reply to chep
    Anonymous:
    John Smallberries:
    Anonymous:

    John Smallberries:

    JavaScript has an indexer for strings, so you can just use:
    n = cfield.value[i];

    RTFM! [8-|]

    In javascript, object String does not have indexer!

    VBScript does.


    RTFM if you want to, but I replaced one line with the other and it worked fine.
        //var n = cfield.value.substr(i,1);
        n = cfield.value[i];

    Yeah! n is always undefined now...  [:)]

    Oh! Wait a second! You are running Mozila-based browser right?

    This is Mozila feature and not a standard of javascript. IE doesn't like it... [;)]

     

  • (cs) in reply to chep
    Anonymous:
    Oh! Wait a second! You are running Mozila-based browser right?

    This is Mozila feature and not a standard of javascript. IE doesn't like it... [;)]

     


    Indeed! What's IE?
    (So mea culpa; empiricism takes another hit.)
    :(

  • (cs) in reply to chep
    Anonymous:
    Anonymous:
    John Smallberries:
    Anonymous:

    John Smallberries:

    JavaScript has an indexer for strings, so you can just use:
    n = cfield.value[i];

    RTFM! [8-|]

    In javascript, object String does not have indexer!

    VBScript does.


    RTFM if you want to, but I replaced one line with the other and it worked fine.
        //var n = cfield.value.substr(i,1);
        n = cfield.value[i];

    Yeah! n is always undefined now...  [:)]

    Oh! Wait a second! You are running Mozila-based browser right?

    This is Mozila feature and not a standard of javascript. IE doesn't like it... [;)]

     



    Actually, if it works in Mozilla, then by definition it is standard JavaScript. It isn't ECMAScript or JScript, but it is JavaScript. It isn't cross-browser, and in a read-only action it does nothing that the cross-browser, ECMA-standard charAt() doesn't also do.
  • (cs) in reply to Stan Rogers

    The biggest (blatant) WTFery is the alert WITHIN the loop.  Bad.

  • (cs) in reply to John Smallberries
    John Smallberries:
    <font size="2">omg!
    reenter should not be hyphenated.</font>


    Wrong! every grammar nazi knows that it should be reënter.
  • (cs) in reply to Charles Nadolski
    Charles Nadolski:
    John Smallberries:
    <font size="2">omg!
    reenter should not be hyphenated.</font>


    Wrong! every grammar nazi knows that it should be reënter.


    Every preschooler knows that one should begin sentences with "Capitalization". *

    *[Placement on the period of that last sentence depends on the country you live in.]
  • chep (unregistered) in reply to Stan Rogers
    Stan Rogers:
    Anonymous:
    Anonymous:
    John Smallberries:
    Anonymous:

    John Smallberries:

    JavaScript has an indexer for strings, so you can just use:
    n = cfield.value[i];

    RTFM! [8-|]

    In javascript, object String does not have indexer!

    VBScript does.


    RTFM if you want to, but I replaced one line with the other and it worked fine.
        //var n = cfield.value.substr(i,1);
        n = cfield.value[i];

    Yeah! n is always undefined now...  [:)]

    Oh! Wait a second! You are running Mozila-based browser right?

    This is Mozila feature and not a standard of javascript. IE doesn't like it... [;)]

     



    Actually, if it works in Mozilla, then by definition it is standard JavaScript. It isn't ECMAScript or JScript, but it is JavaScript. It isn't cross-browser, and in a read-only action it does nothing that the cross-browser, ECMA-standard charAt() doesn't also do.

    Is it documented in JavaScript?

  • (cs) in reply to Simon
    Simon:
    1 - hungarian


    Please please please, let's not start this argument again.  We'll have another MSprogrammers vs Everybodyelse flame war.

    Simon:
    2 - hardcoded character values


    And make this whole travesty worse by having something like LOWER_CASE_A used 50-odd times instead? WTF?

  • (cs) in reply to christoofar
    christoofar:
    Charles Nadolski:
    John Smallberries:
    <font size="2">omg!
    reenter should not be hyphenated.</font>


    Wrong! every grammar nazi knows that it should be reënter.


    Every preschooler knows that one should begin sentences with "Capitalization". *

    *[Placement on the period of that last sentence depends on the country you live in.]


    Ah, but a capital letter doesn't necessarily have to follow an eclamation point ;)
  • Gimpy (unregistered) in reply to chep

    no, VBScript doesn't have a string indexer, at least not something like

      c = s[n]

    or

      c = s(n)

    unless you know of some other syntax that I've not been able to find - you have to do this:

      c = mid(s,n,1)

  • Jon (unregistered)

    I found 17 problems. Most were probably already mentioned, and some are consequences of others:


    1. "bf"IsAlphaNumeric;

    2. function takes a form control, when it should take a string;

    3. function modifies the form control's value;

    4. failure to use a regex;

    5. substr instead of charAt;

    6. flagrantly long "if" statement;

    7. _, @, - and . are apparently alphanumeric;

    8. shows a message box

    9. ... with an incorrect capital letter

    10. ... and a missing word;

    11. modifies the control again

    12. ... forcing the user to retype the entire thing;

    13. focuses the control;

    14. modifies the control again, this time to uppercase

    15. ...on every iteration of the loop

    16. ...after the if statement, which could have been simplified otherwise; and

    17. does not return true or false!

  • chep (unregistered) in reply to Gimpy

    Anonymous:
    no, VBScript doesn't have a string indexer, at least not something like

      c = s[n]

    or

      c = s(n)

    unless you know of some other syntax that I've not been able to find - you have to do this:

      c = mid(s,n,1)

    You're right!

    My bad! [:$]

  • (cs) in reply to chep
    Anonymous:

    Is it documented in JavaScript?



    Yes. See the expanded definition of String in the JS 2.0 standard:

    http://www.mozilla.org/js/language/js20/formal/notation.html#string

    I believe it's also in the ECMAScript 4 Proposal, but that hasn't been published as an ECMA standard yet.
  • Carl (unregistered) in reply to John Bigboote
    John Bigboote:
    Anonymous:
    Anonymous:
    Noone seems to have covered this wtf: The whole logic is mixed in with the page implementation. This wtfery is probably copied and pasted for every form field that needs to be validated.


    The form field object looks to be passed in as  'cfield' so  can actually be used for any form field.

    Although what the 'c' stands for in the hungarianesque notation I don't know.


    Cocktostoen

    This is where I knew my mouth was gonna be...

  • The Submitter (unregistered) in reply to BradC

    Correct.

Leave a comment on “Spot The WTF: JavaScript Edition”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article