• Mickey (unregistered)

    Now that's just terrible.

  • Dirk (unregistered)

    oneGottaLoveReallyLongFunctionNamesBecauseTheyAreSoDescriptive()

  • (cs)

    Shouldn't that method be named like this:

    convertSingleQuoteAndDoubleQuoteUnicodeToAscii

    ?!?

    (gotta do it right if at all)

  • Joshua (unregistered)

    You gotta love how the only two comments in the function are wrong too. &#39 != double quote.

  • (cs)

    Form and function are completely mistaken in this one.

  • freelancer (unregistered)

    ThisIsTheNewFunctionToConvertSingleQuoteAsciiToCharactersActuallyNowItAlsoConvertsDoubleQuoteAsciiToCharacters

  • Looce (unregistered) in reply to Dirk
    Dirk:
    oneGottaLoveReallyLongFunctionNamesBecauseTheyAreSoDescriptive()
    Actually, the teachers I had in computer science all liked long function names; in at least one assignment, I made a really convoluted function name, like createLabelToPlaceAtTheTopOfTheUserInterface(), just to get them to tell me that I needed to remove some words in that.

    Back on topic, though, isn't that an HTML Entity to ASCII converter, and not ASCII to character?

    convertSingleQuoteAndDoubleQuoteHTMLEntityToASCIICharacters() would be a more fitting name for the function... ;)

  • Will (unregistered)

    This method should really be called convertFirstSingleQuoteAndDoubleQuoteAsciiToCharactersAndGoIntoAnInfiniteLoopIfThereAreMore.

  • Kuli (unregistered) in reply to Looce
    Looce:
    convertSingleQuoteAndDoubleQuoteHTMLEntityToASCIICharacters() would be a more fitting name for the function... ;)
    Or findMoreThanOneOfTheSameQuoteCharactersByNotReturningAnyMore().
  • C Gomez (unregistered)

    As a very broad and general rule, once I've descriptively named a method something like that, it tells me I should rethink the method. After all, if it takes that long to describe it, it's possible that it needs to be refactored.

    Not a hard and fast rule but it at least gets me thinking.

  • Cable (unregistered)

    Sometimes I think some programmers try to rewrite some useful code at locations where it would be extremely useless, just to show they are capable at creating garbage that looks nice.

    I think I will have to get some more dried frogpills.

  • dkf (unregistered)

    I especially like:

    tmpIndex = str.indexOf("'");  // double quote
    and
    tmpIndex = tmpName.indexOf("""); // single quote
    There's nothing like a good old misleading comment to set you up for the rest of the code...
  • ok (unregistered)

    I must admit it took me a while to notice said loop. :-.

  • kanna (unregistered) in reply to ok
    ok:
    I must admit it took me a while to notice said loop. :-\.

    You're not the only one :( I'd like to blame my lack of experience with Javascript, but I don't think that would really do anything to mitigate my shame.

  • (cs) in reply to Kuli
    Kuli:
    Looce:
    convertSingleQuoteAndDoubleQuoteHTMLEntityToASCIICharacters() would be a more fitting name for the function... ;)
    Or findMoreThanOneOfTheSameQuoteCharactersByNotReturningAnyMore().
    Let's just cut to the chase and call it thisIsAStupidFunctionWhichShouldNeverBeUsedForAnyPurposeUnderAnyCircumstances(youHaveBeenWarned)
  • Smitty (unregistered) in reply to C Gomez
    C Gomez:
    As a very broad and general rule, once I've descriptively named a method something like that, it tells me I should rethink the method. After all, if it takes that long to describe it, it's possible that it needs to be refactored.

    Not a hard and fast rule but it at least gets me thinking.

    How true, how true, particularly when the thinking get's you to look in the class library and notice String.Replace(). Be careful, this usually coincides with possibly unexpected performance gains as well.

  • martin (unregistered) in reply to Mickey
    Mickey:
    Now that's just terrible.

    no kidding.

    To the author of the original code: Keep your hands in your pockets next time you see a computer, trust me.

  • Michael Houghton (unregistered)

    'How did their data get “'” in the first place?'

    TinyMCE does this in certain circumstances. At least, I find myself having to clean them up, for reasons I am not clear on. It would certainly explain why there was a need to process them in javascript.

  • anne (unregistered)

    Has anyone noticed how many of these involve string processing? I suppose string processing is kind of annoying sometimes, but why does it produce so many glorious WTFs?

    The infinite loop is teh awesome. DoS, anybody?

  • (cs)

    Yes, but it's Self Documenting©

  • (cs) in reply to anne
    anne:
    Has anyone noticed how many of these involve string processing? I suppose string processing is kind of annoying sometimes, but why does it produce so many glorious WTFs?

    I've seen countless coders really infatuated with strings. Doing things like <number>.toString().trim(), which would be toString()'ed AGAIN some lines later, then converted back to numeric. String being like a panacea: not working? .toString() it!

    Being so lovely and overused, it's no wonder why they end up WTF'ed...

  • (cs)

    All things considered, it's probably best that the guy who wrote this doesn't know about regexes.

  • Eam (unregistered) in reply to C Gomez
    C Gomez:
    As a very broad and general rule, once I've descriptively named a method something like that, it tells me I should rethink the method. After all, if it takes that long to describe it, it's possible that it needs to be refactored.

    Not a hard and fast rule but it at least gets me thinking.

    For real, but you also gotta make sure you're documenting what the method accomplishes, not how it works. In this case, a better name might be "UnescapeQuotes"

    Although a WTF by any other name...

  • Anonymous (unregistered)

    It should be named

    replaceCharacterEntities(string)

  • Marcin (unregistered)

    Perhaps I'm being stupid, but what's wrong with this code, other than being redundant? I don't see why it would infinite loop, possibly because I've never tried to manipulate strings in ecmascript.

  • Mystified (unregistered) in reply to TheRider
    TheRider:
    Shouldn't that method be named like this:

    convertSingleQuoteAndDoubleQuoteUnicodeToAscii

    No, it shouldn't.

    Let's all, please, divorce ourselves of the idea that Unicode is a character set, or character representation in any way, shape or form. It is not. Unicode is an attempt at a canonical set of all written glyphs, no matter what language or languages they occur in, with a unique value assigned to each. A character set is a specification of binary representations of a set of glyphs. UTF-X are also NOT UNICODE. They are character sets which provide specifications for the binary representations of glyphs contained in the Unicode canon.

    What this guy was trying to do was convert the HTML representation of the Unicode value of a given glyph to an ASCII character set representation of the same glyph.

    Why is Unicode so hard?

  • DigitalGnome (unregistered) in reply to Marcin
    Marcin:
    Perhaps I'm being stupid, but what's wrong with this code, other than being redundant? I don't see why it would infinite loop, possibly because I've never tried to manipulate strings in ecmascript.

    It took me a shamefully long time to figure it out as well. It's because in his first loop if a 2nd entity is found it will use the original string to replace it. So basically it will flip-flop between the two ' strings converting one to a ' and having the other one reverted back to a '

  • (cs) in reply to Marcin
    Marcin:
    Perhaps I'm being stupid, but what's wrong with this code, other than being redundant? I don't see why it would infinite loop, possibly because I've never tried to manipulate strings in ecmascript.

    Well... take a look at the first loop, and check what happens when a characted index from ONE (mutable) string is used to extract text from ANOTHER (constant) string... That is the problem with tmpIndex, tmpName and str.

  • dolo54 (unregistered)

    function names are apologies?

  • Marcin (unregistered) in reply to H|B
    H|B:
    Marcin:
    Perhaps I'm being stupid, but what's wrong with this code, other than being redundant? I don't see why it would infinite loop, possibly because I've never tried to manipulate strings in ecmascript.

    Well... take a look at the first loop, and check what happens when a characted index from ONE (mutable) string is used to extract text from ANOTHER (constant) string... That is the problem with tmpIndex, tmpName and str.

    Oh good point. I guess the real WTF is that they didn't use functional programming.

  • (cs) in reply to Marcin
    Marcin:
    Perhaps I'm being stupid, but what's wrong with this code, other than being redundant? I don't see why it would infinite loop, possibly because I've never tried to manipulate strings in ecmascript.

    Inside the while loop it keeps working with the str variable instead of the tmpName variable, so on the second pass it reintroduces the first ' into tmpName, then on the third pass reintroduces the second one, on the fourth pass the first one again, etc ad infinitum

  • (cs) in reply to Marcin
    Marcin:
    Perhaps I'm being stupid, but what's wrong with this code, other than being redundant? I don't see why it would infinite loop, possibly because I've never tried to manipulate strings in ecmascript.

    It's easy to overlook, because we tend to see what we want to see. I had to look a couple of times to see it, too, and I only did that because I knew (thanks to comments) that there was an infinite loop.

    Inside the loop (so on all iterations other than the first), the code keeps scanning the original, unmodified string to see if it contains the target sequence. Since it scans the unmodified string, it will always find it.

    When we see what appears to be an iterative scan-and-replace pattern, we assume that the author avoided this bug, either by starting each scan from after the previous hit, or by recursively rescanning the modified input. (The latter is obviously less efficient - O(n**2) in the worst case rather than O(n) - but elegant when implemented in functional style, which may make it preferable in some situations if performance isn't an issue.)

    -- Michael Wojcik

  • that guy (unregistered)

    InPvpAShadowPriestTheFacesWillMelt()

  • (cs)

    Why does the coder insist reproducing Javascript’s String.Replace() function?

    Duh, you don't get LOC bonuses by using libraries! =)

  • benk (unregistered)

    As the code is written, it'll always return the input string. Since tmpIndex = -1 at the top, while(tmpIndex > -1) will never run. Of course, that's probably just a transcription error.

  • benk (unregistered) in reply to benk

    Arg, I'm an idiot. Or really tired. Or both (probably this one).

  • JF (unregistered) in reply to benk
    benk:
    As the code is written, it'll always return the input string. Since tmpIndex = -1 at the top, while(tmpIndex > -1) will never run. Of course, that's probably just a transcription error.

    eh, tmpIndex is assigned a value IF it find the character in the string...

    tmpIndex = str.indexOf("'");

  • Been there done that don't care any more (unregistered) in reply to bob the dingo

    I found something like this once (but with variables - that were set far in advance of the call - instead of literals). The only thing I can figure is that it used to do the replacements, and at some point, they became unnecessary. The subsequent coder didn't realize they were just undoing a preious replace, and the whole thing could have been eliminated, so they just tacked on additional calls to replace to "fix" the broken string.

    String wtf = someString.replace("A","B").replace("C","D").replace("B","A").replace("D","C");
  • JF (unregistered) in reply to JF
    JF:
    benk:
    As the code is written, it'll always return the input string. Since tmpIndex = -1 at the top, while(tmpIndex > -1) will never run. Of course, that's probably just a transcription error.

    eh, tmpIndex is assigned a value IF it find the character in the string...

    tmpIndex = str.indexOf("'");

    Sorry didn't see your 2nd post :) guess I'm tired as well.

  • (cs)

    Just let those punks at Apache do all the hard work for you.

    StringEscapeUtils

  • (cs) in reply to anne
    anne:
    Has anyone noticed how many of these involve string processing? I suppose string processing is kind of annoying sometimes, but why does it produce so many glorious WTFs?

    The infinite loop is teh awesome. DoS, anybody?

    No idea, but you're right, it crops up a lot; or else we see my own personal favourite technique - date processing by means of toString() and string slicing. Yuk.

    Oddly, shortly after I discovered automated unit testing, I tried it on this kind of isolated string processing function, and found that it is is ideally suited to rapidly and completely testing this kind of function. I guess you need to avoid unit testing and refactoring altogether in order to to make this kind of infinite-loop wtf.

  • me (unregistered) in reply to foxyshadis
    foxyshadis:
    Just let those punks at Apache do all the hard work for you.

    StringEscapeUtils

    And how do you call a java class from JavaScript? Ah, sure, just add a web service! That's so enterprisey...

  • Elvis (unregistered)

    Another WTF: tmpIndex will be the offset of &#39 in tmpName which will replace the wrong characters in str the second time through the loop since tmpName is 4 characters shorter than str. Depending upon where the two occurrences of &#39 are would either cause an infinite loop or return with some funky results.

    Consider the string: 'Hi' Expected result: 'Hi' Actual result: &#3'#39;

  • Anonymous Tart (unregistered) in reply to Been there done that don't care any more
    Been there done that don't care any more:
    I found something like this once (but with variables - that were set far in advance of the call - instead of literals). The only thing I can figure is that it used to do the replacements, and at some point, they became unnecessary. The subsequent coder didn't realize they were just undoing a preious replace, and the whole thing could have been eliminated, so they just tacked on additional calls to replace to "fix" the broken string. 
    String wtf = someString.replace("A","B").replace("C","D").replace("B","A").replace("D","C");

    Yeah ... dont maintain any of my code thx. Complete noop innit....

    >>> var a = "BBAA"; var b = "AABB";
>>> a.replace("A","B").replace("C","D").replace("B","A").replace("D","C")
"ABBA"
>>> b.replace("A","B").replace("C","D").replace("B","A").replace("D","C")
"AABB"
  • AdT (unregistered)
    Alex Papadimoulis:
    If you pass ")'Malley''s Irish Pub" it will … actually, never return.

    Hey! Who are you to decide the halting problem? :-)

    Marcin:
    Perhaps I'm being stupid, but what's wrong with this code, other than being redundant? I don't see why it would infinite loop, possibly because I've never tried to manipulate strings in ecmascript.

    The second and all later indices are obtained from tmpName, but the substrings used to reconstruct tmpName are taken from str, which is never modified.

    Thus, if str is 'foo', tmpName will alternate between 'foo' and 'foo', ad infinitum.

    Even if this was fixed, the function would have quadratic worst-time performance, but hey, at least that means runtime is finite.

    There are more gems in this function.

    • Pointless use of temporary variables tmpName and tmpName2
    • Variables named tmpName and tmpName2 that do not necessarily hold a name
    • The ifs don't do anything. Ever. The only reason why tmpName.length would be less than 1 is that str is empty. But then tmpName, which is empty, is overwritten with str, which is empty.
    • str.replace would have worked fine
    • Why write a function that can only replace " and ' instead of all HTML rsp. XML entities?
  • Tom (unregistered) in reply to kanna

    I just didn't expect it to find the replaced "'" character. Too much discipline from Perl regular expressions in my skull to realize how plain silly Javascript is.

  • AdT (unregistered) in reply to Mystified
    Mystified:
    What this guy was trying to do was convert the *HTML representation* of the Unicode value of a given glyph to an *ASCII character set* representation of the same glyph.

    Nonsense! What he was trying to do is converting the HTML entities for single and double quote to ordinary characters. What do ' and " in JavaScript have to do with ASCII?

    Mystified:
    Why is Unicode so hard?

    Good question indeed.

    MichaelWojcik:
    Since it scans the unmodified string, it will always find it.

    No, it doesn't.

    me:
    And how do you call a java class from JavaScript? Ah, sure, just add a web service! That's so enterprisey...

    Just don't forget to XML escape your stuff before sending it to the web service. :-)

  • (cs)

    Wow, that's a weird piece of code.

    I've done some stuff like this myself, but never to this extent. (And, in case anyone is wondering: the legitimate reason to do stuff like this is to make your Javascript run in really old browsers. In this case, it's pointless, because String.replace dates to Netscape 4 and IE 4; nobody's going to expect anything older than that to work anyway these days. If that was the goal, and this was not mere cluelessness, they still made an error: there is some browser version -- I forget which, but it might have been a version of IE for Mac -- you can't assume arguments are proper String objects to begin with and have to explicitly use "str = new String( str )" at the top of your code to make sure the browser knows what you mean.)

  • Todd (unregistered) in reply to Dirk

    drStranglove()||doubleQuoteHowICameToStopWorringCommaAndLoveTheBombPeriodEndQuote()

  • h@x0r (unregistered) in reply to anne

    since this is javascript, you'd only be DoS'ing yourself...

Leave a comment on “convertSingleQuoteAndDoubleQuoteAsciiToCharacters”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article