- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- 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
Klak
Admin
It doesn't excuse the function name or the if condition but I could imagine that "String" used to be some internally-developed string class that was later replaced with the typedef of std::string. I guess whoever did that got it all compiling but didn't clean up the now-redundant methods like this one.
Admin
You call a function that converts all the lowercase letters to uppercase "toLower?" I impressed you came up with a name somehow worse than "intToString."
Admin
So I'm not a c++ dev so sorry if I'm incorrect here, but this looks like a toUpper function to me.
97: a 65: A
p_param[i] = j-32;
p_param[i] = /a/97-32;
Seems that it is taking 32 away from every character from 97 to 122, all the lower case characters.
Result should be 97-32 == 65 /A/
Admin
Yes, indeed, citing it as really being "toLower" is a substantial fail on the part of the editor of the article.
Admin
It all makes sense when you take upper-case digits into account.
Admin
I mean when you take lower-case digits into account.
Admin
The name and the long list of
==checks instead of a couple of</>are just the tip of the iceberg. For me, there are bigger WTFs than those:std::stringor not after.stringconstructor at the end.Admin
Uppercase characters were used first in computing and therefore are smaller numbers in ASCII. At least that;s my theory and it helps me remember that subtracting 32 is a toUpper operation.
I would 'optimize' the code by replacing the assignment with p_param[i] &= 0b1011111;
Admin
TRWTF is the fact that it modifies its input argument, of course.
Admin
It not more toUpperUntilNonPrintable(), because it also terminates the string on a non-printable.
Admin
In my defense, I forgot how subtraction worked for a moment and misread it as adding 32.
Admin
This is what we would call a Fractal WTF. Or WTF2 (squared) for short.
You can peal its layers away like an onion. But no matter how deep you go the tears keep coming.
Admin
Maybe "why?" should be removed from a maintenance programmer's vocabulary. It only leads to drug abuse, alcoholism or voting Libertarian.
Admin
Rather inPlaceToUpperAndReturnUntilNonPrintable(), since this function de facto has two outputs.
Admin
Aside from the more obvious things wrong, just look at the arguments. Both have a sort-of Hungarian prefix on them. That leaves the first argument as "param", which tells us nothing about the purpose of the argument. The second argument also has a "p_" prefix, despite the length parameter being an int, and not a pointer.
Admin
I'm guessing that this version of Hungarian decorates with "p_" to mean "parameter", which I find absolutely hilarious.
Admin
"I would 'optimize' the code by replacing the assignment with p_param[i] &= 0b1011111;"
This would require another load of p_param[i] that's already in variable j which is probably assigned to a CPU register.
Also, bitwise AND is no improvement over subtraction. I've worked on CPUs that had two arithmetic units but only one AND/OR/NOT unit, so it could execute two subtractions in a cycle but only one AND.
Admin
It would be more useful to remove "why not?".
Admin
If it's not declared 'const', then that's acceptable and should be expected. Maintaining const-correctness in a codebase is something that in my experience is sorely overlooked.
Admin
This is something that I first encountered developing for Symbian (or EPOC as it was at the time) - https://silo.tips/download/symbian-os-c-coding-standards
It's not Hungarian (which I dislike) - it only takes one person to change the declared type of something without changing all of the references to the name and now the compiler and the maintenance programmer are reading very different things.
Instead, it's a scope indicator - in Symbian, "aThing" is an argument to a function, "iThing" is a member variable in the object instance, and there were lots more (so, "iThing = aThing;" was obviously just copying the function argument to the member variable without thinking of a different name - which makes constructors very easy to grok. In C++ where 'foo' might be a local variable, argument, instance variable, something in an active namespace etc, this actually made things much more understandable.
The upper-case version is a more of a "capability" thing. Functions which could do the equivalent of throwing an exception (Symbian had a thing called a "Cleanup Stack" because C++ exception handling wasn't very well supported at the time) started with a 'C' - "CMyFunction". So, if you had a function that didn't start with a C but called functions which did, that was an obvious bug right there ("I'm calling things that can throw an exception, but my name does not indicate that I will throw an exception"). A fairly elegant solution at the time to an issue that is mostly handled by more capable tools these days.
Admin
"So, intToString takes a buffer of bytes and a length for the buffer, and at a glance, you'd just assume that someone is reinventing atoi."
Given that the name is "intToString" and the return type is String, I would not assume that this is atoi (or "stringToInt") :D
Admin
Unless you're running it on an IBM mainframe, in which case its a lossy monoalphabetic substitution cipher function that swaps EBCDIC values around.
Admin
Should I have used double quotes instead of single quotes around the word 'optimize' so it would have been more apparent?
Admin
People keep forgetting the original intent of Hungarian notation, what is now called "Application Hungarian" (as opposed to "Systems Hungarian").
To remind everyone...
Systems Hungarian is the thing everyone thinks that "Hungarian notation" means, where the wart indicates the compiler's view of the type (i for int, f for float, etc.), and is rightly criticised for what you cite, among other things.
Application Hungarian is Charles Petzold's (the Hungarian in question) original vision, where the wart indicates the "flavour" of the data in the variable, to make up for deficiencies in the language's type system. So for string variables (e.g.
char *in C - remember, the type system is deficient) might have one of the following warts:html== HTML-encoded textplain== plain text, not HTML-encoded or whatevercipher== encrypted ciphertextAnd correct calls to conversion functions resemble:
html_new_page = html_from_plain(plain_new_page);=> converts plain text containing & and < and > and other ugliness into the HTML-encoded versionplain_received_data = plain_from_cipher(cipher_received_data);=> decrypts the ciphertextAnd wrong calls to conversion functions resemble:
plain_new_page = html_from_plain(cipher_data);=> "plain" doesn't match "html" in the assignment, and "plain" doesn't match "cipher" in the argument.It's largely obsolete these days in fully type-enforcing languages, since we use more elaborate type systems (C++, Cflat, Java, etc.) to have the compiler rather than the reviewer's eye enforce the right calls.
Systems Hungarian is an abomination no matter which language you're using.
The Windows API is littered with examples of both, of course.
Admin
Indeed, I missed that, too. It should be
itoa.Admin
Small correction: Charles Simonyi.
Admin
Interesting - thank you for educating us :) It seems that Systems Hungarian is what I've been exposed to - and yes, it was the cause of totally avoidable bugs in Win32 code where someone checked in a change with a type/name mismatch and people later believed the name without checking the type.
Admin
Ugh, yes. Thanks for the correction.
Admin
Perhaps, he is thinking of PETSCII?
Admin
'i' is uninitialized because it is later set by the 'for'. 'j' is initialized despite being later set in the 'for'. Both should have been declared & initialized in said 'for'.
A detail is never too small to sneak in inconsistencies and creep.