- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
"Paul adds: every string in the system is routed through these functions and accessed via an ObjectFormatter class with a single method (“Format”) which has a single argument (“Object obj”). The Format method is called via remoting, looks up the object’s type in an XML configuration file, and calls the configured formatter via reflection."
It's like the ultimate Enterprise Application!
Admin
Admin
So, what does the function do? <g>
Admin
Can somebody tell that what are the special chars except quote, ampersand, apostrophe, open bracket, close bracket, comma, hyphen, full stop, comma and forward slash? (I suppose comma is mentioned twise in the list just to make sure.) My brain blew up when I was trying to find out the characters which the code removes. That code could be written with one regexp replace command.
Admin
I'm rather surprised by the characters the routine removes, but I suppose there is a good business reason for that. Or it could be totally random, but I frequently find it hard to tell those two apart anyway...
So what purpose does this serve?
Admin
That' sick.
Captcha: cognac - Ex-cat-ly what I need now...
Admin
... Ex-act-ly of course ...
Should have that Cognac now.
Admin
To establish a real efficient bottleneck, this has to be synchronized.
Admin
Despite the long function name this is not entirely what this function does, consider the else.
the this is converting a 96 to a 39. The function name is wrong.
Admin
That would be even sillier. Regular Expressions would seldom be easier to understand, and they are an order of magnitude slower. But of course, the routine shown here is questionable.
Admin
"Paul adds: every string in the system is routed through these functions and accessed via an ObjectFormatter class with a single method (“Format”) which has a single argument (“Object obj”). The Format method is called via remoting, looks up the object’s type in an XML configuration file, and calls the configured formatter via reflection."
That's the only real WTF, no excuse for it. The method's implementation is not optimal but no WTF either. The method's spec sounds reasonable - remove input characters that could cause problems by allowing only those deemed safe (i.e. not the "ennumerating badness" antipattern). OK, the name is idiotic too. But the above, that's WTF gold.
Admin
Umm...numbers (codes 48-57) are special characters now?
I'm guessing that the XML has no dates, times, telephone numbers, postal codes, SSNs, indexes, hashes, ISBNs, UPCs, RFCs, cube assignments, irrational numbers....
Admin
And you missed the (chrCode==39 in the "if" part so this will only fire if it is a 96 - converting a ` into a '.
Admin
The else if checks for char 39 and 96 but 39 will be caught by the if so it'll never get caught in the else if.
If you are forced to write such a retarded method you should at least do it properly.
CAPTCHA = B0RKX0rZZ!!11!!112!!!!1ONE!1!!!!!LOL!!11!!ROFLCOPTER!!111
Admin
s/[^-&'(),./0-9A-Za-z]//;
How's that hard to understand? ;-)
Admin
No, that is an or condition. || means or && means and.
You played yourself.
Admin
eerr... what about "RemoveInvalidCharacters"?
I personally use "ReplaceShit" in my libraries. Dunno, maybe a WTF. But i like that way.
Admin
The goggles, they do nothing!
Admin
I read it that way, too, but I think what Zemm meant was that 39 is in the first part of the if, so it shouldn't ever be true in the else-if anyway.
Admin
At least they're using a StringBuilder
Admin
Does the 'else if' check for 39? Yes. Does the 'else if' check for 96? Yes.
Therefore you can say the 'else if' checks for 39 and 96. There is a language outside of computers called english that some of us like to use.
The point still stands that if the chrCode is 39 it'll never make it to the 'else if'.
captch: COCK
Admin
Now you know it's time to leave!
Admin
Hey, I think I found a bug: they don't check for character code 44 twice in the if statement. According to the function name this must be checked twice.
Very "stinky".
Admin
Regular expressions are not slower for this task than remote method invocation and reflection through XML.
Admin
Not to mention that characters 0 through 14 are not special characters...
Admin
Admin
What happens if (or rather, WHEN) new exception characters need to be added? Do they refactor all their code so their function name remains accurate?
Admin
Both of which would still probably be needed in this design. He DID say that everything called this, so your choice is to duplicate that RegEx everywhere, add another component to all the apps/components that call this, or skinny down this code to use the RegEx.
Put another way, the choice of using a RegEx solves very little of this WTF.
Admin
Admin
lol I've wrote functions like this before
Admin
The only WTF is that Paul argued for an hour or two. It would have taken far less time to implement this version, a version that uses a regular expression, and some benchmarking test code.
It really doesn't matter which version the boss picks because the performance gain from the regular expression is going to be overwhelmed with the time it takes to do the remote call.
Admin
I know you're heart's not in it, but at least use switch for readibility.
Admin
Admin
And people still ask me what's wrong with escaping. And I still wonder why, if people have to escape text, they still regularly opt for having more than one escape character.
Admin
'remove/replace chosen characters' regexps start being faster than corresponding series of ||'ed comparisons starting with a series of 8 or so comparisons.
The code is horrible for using magic numbers without any kind of comment. So tomorrow you decide you don't need the ampersand, so you... look up its charcode in the ascii table and seek the corresponding entry in the code?
If you absolutely, positively have to use 'magic numbers', COMMENT! || (chrCode > 64 && chrCode < 91) // A-Z || (chrCode == 34) // " || (chrCode == 38) // & || (chrCode == 39) // ' || (chrCode == 40) // ( || (chrCode == 41) // ) || (chrCode == 45) // -
Also, don't use X > Y-1 && X < Y+1 when you mean X <= Y, X >= Y. I check chrCode > 64, 64='@'. Wtf is chrCode > '@' && chrCode < '[' ? Wouldn't chrCode >= 'A' && chrCode <= 'Z' be better?
The general idea of the procedure is a WTF, sure, but the real WTF is horrible execution of said retarded idea, resulting in code that is WTF Squared.
Admin
Here's another nice thing: "special" characters include everything that doesn't have a character value from 0 through 255. Which is almost, but not entirely, quite unlike ASCII. (Strings in .NET are Unicode, so it's not Latin-1 either.)
The "enterprisey" part is a WTF, but so is this. For the record, yes, a compiled regex would be faster, and I for one would find it vastly easier to read -- what you've got here can only be understood by virtue of the ridonkulously long function name.
One regex to remove undesired characters plus a call to String.Replace() to replace ` with ' (which is what the second if does) would be all it takes.
Some things are not comfortably done with regexen, but this is practically a textbook application.
Admin
But WTF do they do with chars between 15 and 31? They aren't even printable.
Admin
Holy crap! Adding bad code to an all ready rotting code base is a daily requirement! Add to that the requirement of doing everything one to three "billable" hours and you have my job in a nutshell.
Admin
They remove them. WTF, did you not read the code?
Admin
Ahmmm, I'm sorry? Who did not read it?
Admin
Admin
one suspects it might be several times faster not to mention clearer to have a global array that has the character mappings and do it all in one line, something like:
Admin
I'd use a character lookup table which would allow arbitrary character mappings (96 -> 39) and removal.
It'd be a lot quicker than this (less conditional jumps - just 2, one for the loop and one to check for the 'remove character' code) and easier to change rules.
You could even pass different lookup tables to the function for different rules then (but you'd have to change the function name, and call the lookup table 'RemoveSpecialCharsExcept...').
But, I suppose it wouldn't be Enterprisey enough.
(I'm still not entirely sure WHY you'd want to do this though...)
Admin
Drat, someone bet me to it by seconds...
Admin
Admin
In C you can just use a char constant as an integer.
Instead of (chrCode==34) you can say (chrCode=='"'). Likewise for the other characters.
Admin
I think this is the case where an ALT+0096 (tick mark) is placed instead of a single quote. (ALT+0039)
96 = ` 39 = '
See the difference? Maybe that is part of the special characters they want to munge.
Admin
y#A-Za-z0-9/.,()'&-##d;
Admin
TRWTF is that this site doesn't use a CSS width specification with scroll bars where necessary, rather than mangling the code so it doesn't compile.
Admin
I guess you're being a bit sarcastic, but I find that pretty easy to read. If you know the purpose of a regex, deciphering it becomes much easier because there are really only a few sensible ways to write it.
Only thing I would change is to add the global flag:
s/[^-&'(),./0-9A-Za-z]//g