• (cs)

    brillant! (its obligatory now, right?)

  • Matt (unregistered)

    This isn't a WTF... they want to allow all sorts of SQL injuection attacks, bad characters, and what not into their addresses...

    I wonder what the specification for this crap looked like!

  • (cs)

    Holy crap.  Seriously, what the heck are people thinking sometimes?  He understands char arrays, he understands some basic string manipulation, but he doesn't seem to see a connection between char arrays and strings.  Hmmmm...

    I also think this should win some award for most innefficient algorithm EVAR.

    Not really a WTF, but stuff like this drives me nuts: 

    ...
    vsadr1Count = vsadr1Test.length();

    <font>for(</font> <font>int </font>i = 0; i < validSignCount; i++ )
    ...


    Why the crap do some people create an extra variable for the heck of it?  I don't know if Java has to do some funky calculations to determine the length, or if it's just a maintained property, but still, just use the damn getter!
  • (cs)

    Well, if this language is the same as we've been seeing lately, then this case looks tailor made for REGEX.

  • (cs) in reply to Ytram

    I quoted the wrong for loop, I was actually referring to the second loop that uses the newly created variable.  You get the picture...

  • (cs)

    That way he doesn't need to include the space character in validSign (sic). I think one point deserves praise: He, unlike many other US-American programmers, thought of us foreigners and our funny accented characters (well, ok, at least he thought of us non-Scandinavian non-Greek non-Kyrillic-using Europeans). He was even nice enough to include a lowercase ß and an uppercase ß for people from the German-speaking countries. Never mind that there's no such thing as an uppercase ß, though.

  • MurphyMan (unregistered)

    Now this is said a lot, but this is truly the best I've seen in a while.

    Where to start? The char array is ... er... strange but maybe they're very specific about the characters they allow. But the same method repeated 5 times? Returns an int? Takes a StringBuffer as an argument but only ever uses a String? Throws that lovely ole generic 'Exception'? Returns 0 if the StringBuffer is null? Those lovely lower-case method names?

    And somehow my shipping address can contain percent signs, equals signs, parentheses, and question marks - but numbers are just out of the question.

    Just... wow.

  • MurphyMan (unregistered) in reply to MurphyMan

    Oopps.. missed those numbers in the array there. I take it back, give the guy a raise

  • (cs)

    Now that I think of it, given that the order of non-ASCII characters is so chaotic, I'm surprised there aren't more dupes.

  • Rich C. (unregistered)

    HAHA!

    Obviously the guy is an id10t... It's apparent that instead of replacing the char with ' ' in the first loop he should have replaced with ''.

    Then (and this is brilliant!) he could have replaced the 2nd loop with:

    if len(wtf_replaced_string) > 0
       throw exception...


    it would save a whole nother iteration through the string...

    seriously, how can I burn the memory of this from my brain???



  • drigz (unregistered) in reply to MurphyMan
    Anonymous:
    And somehow my shipping address can contain percent signs, equals signs, parentheses, and question marks - but numbers are just out of the question. Just... wow.


    Numbers are allowed.
  • (cs)
    Alex Papadimoulis:
        <font color="#000099">for(</font> <font color="#000099">int </font>i = 0; i < validSignCount; i++ )
    {
    vsadr1Test = vsadr1Test.replace( validSign[ i ], <font color="#990000">' </font><font color="#990000">' </font>);
    }

    <font color="#000099">for(</font> <font color="#000099">int </font>i = 0; i < vsadr1Count; i++ )
    {
    <font color="#000099">if(</font> vsadr1Test.charAt( i ) != <font color="#990000">' </font><font color="#990000">' </font>)
    {
    <font color="#000099">throw </font><font color="#000099">new </font>VerificationException(
    <font color="#990000">"Vsadr1 with invalid sign"</font>,
    VerificationException.VSADR1_INVALID_SIGN );
    }
    }



    This is for performance, since 2 for loops = O(2n) ~= O(n), as opposed to nesting the for loops for O(n^2)!!

    And yes, that is sarcasm, I realize that replace() must loop through the string as well, creating O(n^2) anyway.

  • drigz (unregistered) in reply to drigz

    OK, you noticed.

  • Volmarias (unregistered) in reply to Ytram

    REGEX? I'd be content with an ascii table to look at, and a few if statements using the data from it.

  • (cs) in reply to kipthegreat
    kipthegreat:

    And yes, that is sarcasm, I realize that replace() must loop through the string as well, creating O(n^2) anyway.



    It's O(nm), where n is the length of the input and m is the number of characters in "validSign". Assuming that the latter is constant, it's O(n) really. Just a very slow version of O(n).

  • (cs) in reply to Volmarias
    Anonymous:
    REGEX? I'd be content with an ascii table to look at, and a few if statements using the data from it.


    This assumes ASCII encoding, though, and given that validChars contains non-ASCII characters, that doesn't seem to be a safe assumption.

  • (cs) in reply to Alexis de Torquemada
    Alexis de Torquemada:
    kipthegreat:

    And yes, that is sarcasm, I realize that replace() must loop through the string as well, creating O(n^2) anyway.



    It's O(nm), where n is the length of the input and m is the number of characters in "validSign". Assuming that the latter is constant, it's O(n) really. Just a very slow version of O(n).



    Yeah I thought of that just after I hit post.. :)

  • (cs) in reply to Alexis de Torquemada

    I hereby revoke this guy's Ctrl+C, Ctrl+V privileges.

    Seriously, cutting and pasting code is the root of some of the worst WTFs I've ever seen. I think the efficiency boost from copying pre-existing code is hardly enough to offset the huge maintenance cost of fixing bugs and stupid code that's been copied and pasted all over the place. Typing every line of code by hand at least forces you to have some understanding of what your code is doing.

     

  • Mike Weller (unregistered)

    Holy shit.

  • Joe (unregistered) in reply to Ytram
    Ytram:
    Holy crap.  Seriously, what the heck are people thinking sometimes?  He understands char arrays, he understands some basic string manipulation, but he doesn't seem to see a connection between char arrays and strings.  Hmmmm...

    I also think this should win some award for most innefficient algorithm EVAR.

    Not really a WTF, but stuff like this drives me nuts: 

    ...
    vsadr1Count = vsadr1Test.length();

    <FONT size=+0>for(</FONT> <FONT size=+0>int </FONT>i = 0; i < validSignCount; i++ )

    ...



    Why the crap do some people create an extra variable for the heck of it?  I don't know if Java has to do some funky calculations to determine the length, or if it's just a maintained property, but still, just use the damn getter!

    Actually, it's a performance optimization.  If you have a large array of elements to iterator through and you have to do it often, then you are going to slow down your loop by using the property 'getter'.

    As the loop terminator expression is evaluated, it ends up becoming a function call and a comparison each time through the loop, as opposed to just a comparison on the stored length.  I commonly use the following in C#:

    for (int index = 0, count = someArray.Count; index < count; index++)
    {
       // work in the loop here
    }

    The property getter is only called once and then the loop iterator compared to the saved value.

  • (cs) in reply to A Wizard A True Star
    A Wizard A True Star:

    Seriously, cutting and pasting code is the root of some of the worst WTFs I've ever seen. I think the efficiency boost from copying pre-existing code is hardly enough to offset the huge maintenance cost of fixing bugs and stupid code that's been copied and pasted all over the place.


    To misquote the Java slogan:  Write once, screw up everywhere!

  • (cs) in reply to A Wizard A True Star
    A Wizard A True Star:

    I hereby revoke this guy's Ctrl+C, Ctrl+V privileges.

    Seriously, cutting and pasting code is the root of some of the worst WTFs I've ever seen.



    That's such a good point. If you have to do it more than a couple of times (maybe even once), make it into a reusable method.
  • (cs) in reply to Dustin

    Okay, I think that I should change my screen name to “Exception Nazi.” For real. This keeps coming up. Why use an exception if it's not an exception. It's an error, just handle it. Give the user a nicely formatted “Hey, you can't do that” message or something.

  • (cs) in reply to loneprogrammer
    loneprogrammer:
    A Wizard A True Star:

    Seriously, cutting and pasting code is the root of some of the worst WTFs I've ever seen. I think the efficiency boost from copying pre-existing code is hardly enough to offset the huge maintenance cost of fixing bugs and stupid code that's been copied and pasted all over the place.


    To misquote the Java slogan:  Write once, screw up everywhere!



    I've always heard it misquoted:  Write once, debug everywhere!
  • Lebedejev (unregistered) in reply to Joe

    Ytram, you are wrong.

    If you write

    for (int index = 0; index < someArray.Count; index++)
    {
    }

    C# compiler makes the optimization automatically for you, so the getter is not called in every iteration. So you don't need to do this manually.

  • Lebedejev (unregistered) in reply to Lebedejev

    Oh sorry, the name should have been Joe [:D] Joe was quoting Ytram and I overlooked Ytram wasn't the author. Sorry Ytram [A]

  • Daniel (unregistered) in reply to Joe
    Anonymous:
    ....

    As the loop terminator expression is evaluated, it ends up becoming a function call and a comparison each time through the loop, as opposed to just a comparison on the stored length.  I commonly use the following in C#:

    for (int index = 0, count = someArray.Count; index < count; index++)
    {
       // work in the loop here
    }

    The property getter is only called once and then the loop iterator compared to the saved value.



    What is someArray? If it's an array, I would use
    for (int index = 0; index < someArray.Length; index++)
    because C# compiler + JIT optimizer recognize this as iteration through an array, automatically put someArray.Length in a local variable and it will skip the checks for array bounds since the value of index is guaranteed to be in the valid range. (the same optimization is done for "foreach" on arrays)
    However, if someArray is an ArrayList, assigning Count to a local variable first is a bit faster than computing the value every time; and it's a lot faster than "foreach" in this case.
  • Arachnid (unregistered)

    For the real WTF, just view the HTML source of today's entry.
    Font tags, anyone?

  • (cs) in reply to Lebedejev
    Anonymous:

    If you write

    for (int index = 0; index < someArray.Count; index++)
    {
    }

    C# compiler makes the optimization automatically for you, so the getter is not called in every iteration. So you don't need to do this manually.

    Depends on what type of variable someArray is. The compiler may be able to optimize out the bounds checking, but then again it may not. For simpler types, yes, you're correct. For more complex types, you're not. So it's usually a good idea to optimize your loop exit conditions manually in any case.

  • (cs)

    You know, I understand that there are people in this world who are actually brain-damaged enough to come up with something like this.

    What I don't understand, however, is how they actually manage to produce something that compiles and works correctly.  The extra variables and redundant loops (I'm quite sure that Java has an IndexOf() method on Strings) are one thing, but it seems to me that anyone who's too dumb to realize that they're writing the same function five different times, with one single difference, should also be to dumb to produce anything that javac would even accept, let alone work correctly.

    This code makes my head hurt.

  • (cs)

    Well, at least he didn't make an array of every possible invalid character and do the testing against that....

  • (cs) in reply to A Wizard A True Star
    A Wizard A True Star:

    I hereby revoke this guy's Ctrl+C, Ctrl+V privileges.

    What about his Ctrl+Ins, Shift+Ins priveleges?

    /Numlock? What's that?

     

     

  • Joe (unregistered) in reply to Lebedejev

    Anonymous:
    Oh sorry, the name should have been Joe [:D] Joe was quoting Ytram and I overlooked Ytram wasn't the author. Sorry Ytram [A]

     

    Actually, I am not wrong, in fact, I can offer proof.  Given the following function:

            static void LoopTestOne()
            {
                ArrayList list = new ArrayList();

                for (int index = 0; index < list.Count; index++)
                {
                    Console.WriteLine("Loop 1");
                }
            }

    I compiled a "Release" version of the assembly, then using Lutz Roeder's Reflector, examined the generated IL.  You see, on L_001a, that the compiler generated get_Count() accessor is called and then the "branch if less than" instruction is used to make the comparison and loop if necessary.

    .method private hidebysig static void LoopTestOne() cil managed
    {
          // Code Size: 34 byte(s)
          .maxstack 3
          .locals (
                [mscorlib]System.Collections.ArrayList list1,
                int32 num1)
          L_0000: newobj instance void [mscorlib]System.Collections.ArrayList::.ctor()
          L_0005: stloc.0
          L_0006: ldc.i4.0
          L_0007: stloc.1
          L_0008: br.s L_0018
          L_000a: ldstr "Loop 1"
          L_000f: call void [mscorlib]System.Console::WriteLine(string)
          L_0014: ldloc.1
          L_0015: ldc.i4.1
          L_0016: add
          L_0017: stloc.1
          L_0018: ldloc.1
          L_0019: ldloc.0
          L_001a: callvirt instance int32 [mscorlib]System.Collections.ArrayList::get_Count()
          L_001f: blt.s L_000a
          L_0021: ret
    }

    I contend that this is for a very good reason.  After all, the value of a property is non-deterministic; most especially in a case where you may have had a secondary thread updating your collection.  In this case, you most definitely want the compiler to be calling your accessor, because the number of elements may have been changed.  In cases where there is a simple loop, the compiler does not optimize away the call to the accessor and rightfully so.  So, if you aren't using your collection amongst multiple threads, and you don't want the overhead of calling your accessor each time the expression for terminating the loop is evaluated, then you will need to optimize this out using the technique that I had noted.

  • Carsten H (unregistered) in reply to Alexis de Torquemada
    Alexis de Torquemada:
    That way he doesn't need to include the space character in validSign (sic).


    I believe that validSign is Germlish. Translated through German, sign = Zeichen = character.

  • (cs) in reply to Oliver Klozoff
    Oliver Klozoff:
    You know, I understand that there are people in this world who are actually brain-damaged enough to come up with something like this.

    What I *don't* understand, however, is how they actually manage to produce something that compiles and works correctly.  The extra variables and redundant loops (I'm quite sure that Java has an IndexOf() method on Strings) are one thing, but it seems to me that anyone who's too dumb to realize that they're writing the same function five different times, with one single difference, should also be to dumb to produce anything that javac would even accept, let alone work correctly.

    This code makes my head hurt.


    Most of these people write something, compile, get 50 errors, and stick something in there to fix each error, one by one, and, once they've achieved the "it compiled!" milestone, don't go back and say "now does this make sense".  I had the misfortune of being stuck on group projects with several of these people in college, who I can only assume completed their degrees only by copying other people's code.  Fortunately I work at a company that has managers with technical backgrounds that can interview decently, so I haven't had to work professionally with any of these people (yet.. I've been out of college less than two years though..).

    One of my friends was working on a project with one of these clueless people (let's call him Paul), and Paul was at the keyboard.  My friend says "okay, now we need to make a for-loop to go through that array", and he types "<font size="2">for (</font>" and then sits there waiting on some kind of instruction as to what to do next.  Not to mention that the guy--junior year of a CS curriculum at a major university--did not know how to type without looking at the keys and using only his index fingers.
  • Joe (unregistered) in reply to Daniel
    Anonymous:
    Anonymous:
    ....

    As the loop terminator expression is evaluated, it ends up becoming a function call and a comparison each time through the loop, as opposed to just a comparison on the stored length.  I commonly use the following in C#:

    for (int index = 0, count = someArray.Count; index < count; index++)
    {
       // work in the loop here
    }

    The property getter is only called once and then the loop iterator compared to the saved value.



    What is someArray? If it's an array, I would use
    for (int index = 0; index < someArray.Length; index++)
    because C# compiler + JIT optimizer recognize this as iteration through an array, automatically put someArray.Length in a local variable and it will skip the checks for array bounds since the value of index is guaranteed to be in the valid range. (the same optimization is done for "foreach" on arrays)
    However, if someArray is an ArrayList, assigning Count to a local variable first is a bit faster than computing the value every time; and it's a lot faster than "foreach" in this case.

    I probably should have specified that "someArray" is a collection class, just like an ArrayList.  In fact, that's what I used in my example.  The compiler doesn't actually assign it to a local, but uses the IL "ldlen" instruction to get the length of the array type on the stack as an unsigned integer.

  • (cs)

    The idea of maintaining this retarded moron's code makes me want to die.

  • (cs)

    But three lefts do...

    WTF!

     

  • Arachnid (unregistered) in reply to Ytram

    For null-terminated strings, checking the length in the loop condition turns it from an O(n) operation to an O(n^2) operation, too, as it has to count the length of the string each iteration. That is, unless your compiler optimises it out, which, as shown, isn't always the case, because it's hard for the compiler to know if the function being called is immutable or not.

  • (cs) in reply to Miszou
    Miszou:

    /Numlock? What's that?



    Fortunately, boot-up numlock can be disabled in many BIOSes, or the OS if necessary.

  • (cs) in reply to kipthegreat
    kipthegreat:
    Not to mention that the guy--junior year of a CS curriculum at a major university--did not know how to type without looking at the keys and using only his index fingers.


    I find this irritating, too, in a programmer, but it seems some people are remarkably proficient at hunting & pecking
  • (cs) in reply to Arachnid

    Anonymous:
    For the real WTF, just view the HTML source of today's entry.
    Font tags, anyone?

    A lot of RSS readers support only font tags ... plus the richtextbox editor used by the forums software is a bit off, too ...    

  • Arachnid (unregistered) in reply to Alex Papadimoulis
    Alex Papadimoulis:

    Anonymous:
    For the real WTF, just view the HTML source of today's entry.
    Font tags, anyone?

    A lot of RSS readers support only font tags ... plus the richtextbox editor used by the forums software is a bit off, too ...    



    Maybe, so, but what rendering information does:
    <FONT>'A</<span class="end-tag">FONT><FONT>',</<span class="end-tag">FONT> <FONT>'B</<span class="end-tag">FONT><FONT>',</<span class="end-tag">FONT>

    etc convey? I only noticed because Google Reader has trouble rendering this - it ended up putting each array element on its own line.

    And I'd submit that even if there are readers that only support the font tag, that's their problem, and it's not up to anyone generating an RSS feed to cater to such a restricted set of functionality. Either use plain text or proper HTML. The font tag has been deprecated for nearly 10 YEARS longer than RSS has existed, and supporting it only encourages more poorly written readers.

  • Arachnid (unregistered) in reply to Arachnid

    That should be "The font tag has been deprecated for nearly 10 YEARS, longer than RSS has existed...". And the forum software (unsurprisingly) munged the opening part of each font tag.

  • (cs) in reply to Joe
    Joe:
    I commonly use the following in C#:

    for (int index = 0, count = someArray.Count; index < count; index++)
    {
       // work in the loop here
    }

    The property getter is only called once and then the loop iterator compared to the saved value.

    What are you writing in C# that makes optimizations like that worth the typing?  I gotta say, that looks like some premature optimization to me.

     

  • llxx (unregistered)

    So it's basically replacing all the valid characters with spaces and then seeing if there's anything that isn't a space?<p>It is not easy to not misunderstand a nonshort string of nonnegatives.<p> Nonetheless, this approach is useful when the number of valid characters is less than the number of invalid ones, as the table is shorter. However, using 5 separate and nearly identical functions is the big WTF.<p>The "accepted" way would be to loop through each character of the input and compare it with entries in the invalid-character table. If one is found, don't throw an exception... do something else.

  • (cs)

    According to the "valid signs", my address here in Albania would be:

    "_;)_:)_:(_:\_:?"

    Wow! I hope they add graphics to the address, too!

    By the way, which are the "invalid signs"?

    <FONT size=+0>
    </FONT> 

  • Tosser (unregistered) in reply to Rich C.
    Anonymous:
    HAHA!

    Obviously the guy is an id10t... It's apparent that instead of replacing the char with ' ' in the first loop he should have replaced with ''.

    Then (and this is brilliant!) he could have replaced the 2nd loop with:

    if len(wtf_replaced_string) > 0
       throw exception...


    it would save a whole nother iteration through the string...

    seriously, how can I burn the memory of this from my brain???





    You should think about what you are saying before you call somebody an idiot. Changing the characters to a NULL value and then checking the length of the string will always accept the address as long as the first character is valid. If the second (or third...etc) is not valid your code would have accepted it as valid.

    Hope you don't code review too often.
  • (cs) in reply to Tosser

    Anonymous:
    Anonymous:
    HAHA!

    Obviously the guy is an id10t... It's apparent that instead of replacing the char with ' ' in the first loop he should have replaced with ''.

    Then (and this is brilliant!) he could have replaced the 2nd loop with:

    if len(wtf_replaced_string) > 0
       throw exception...


    it would save a whole nother iteration through the string...

    seriously, how can I burn the memory of this from my brain???





    You should think about what you are saying before you call somebody an idiot. Changing the characters to a NULL value and then checking the length of the string will always accept the address as long as the first character is valid. If the second (or third...etc) is not valid your code would have accepted it as valid.

    Hope you don't code review too often.

    No, you should think before...
    Replacing all the chars with null, not only the first one...
    If Length(String) > 0
    Returns false! NOW! GO!

  • Tosser (unregistered) in reply to Savior
    Savior:

    Anonymous:
    Anonymous:
    HAHA!

    Obviously the guy is an id10t... It's apparent that instead of replacing the char with ' ' in the first loop he should have replaced with ''.

    Then (and this is brilliant!) he could have replaced the 2nd loop with:

    if len(wtf_replaced_string) > 0
       throw exception...


    it would save a whole nother iteration through the string...

    seriously, how can I burn the memory of this from my brain???





    You should think about what you are saying before you call somebody an idiot. Changing the characters to a NULL value and then checking the length of the string will always accept the address as long as the first character is valid. If the second (or third...etc) is not valid your code would have accepted it as valid.

    Hope you don't code review too often.

    No, you should think before...
    Replacing all the chars with null, not only the first one...
    If Length(String) > 0
    Returns false! NOW! GO!



    If the first character is valid and is replaced with a NULL character, then it doesn't matter what else comes after it the length will always be 0 in otherwords the exception would not be thrown whenever the first character is valid regardless of anything else in the string.

Leave a comment on “Five Wrongs Don't Make A Right”

Log In or post as a guest

Replying to comment #46388:

« Return to Article