- 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
Someone writing comments such as "Should I crash or not?" must use the absolute worst software in the world to think that crashing is acceptable, and is done on purpose.
But that was by far the funniest part of the code. Many CS students simply cannot code so the rest doesn't surprise me at all. Hell, we had to write a Java pretty-printer in school (in Perl) and the teacher's solution made me go "WTF?!". It was tailored to her test cases. Those of us who actually did it properly (so it would work with programs on one line, for example) were utterly pissed off that we spent so much time on it. The students were the same, complaining that they didn't get Perl and didn't know how to write it. ... little did they know that the next assignment was the same app, but in C. snicker
Admin
I'm surprised by the quality of the comments. He even explained that magic 1.
Admin
And so exactly where does c or c++ come in? It does return a default and that is unfortunately chosen (I think it should crash, or at least it should consider that possibility), but it would work the same in pretty much any language supported a switch-like thing.
Admin
It stopped being cool to dog on the name of this site abouuuut a month ago.
Admin
not always true. at my school the first course in software engineering is typically taken the beginning of the 2nd year. most of my peers were terrible programmers after 5 years of school, imagine them after one.
besides, most software engineers i know are reasonably competent but not outstanding programmers. i'm guessing this is because there is much more emphasis on the entire process rather than just writing efficient, clean code. of course this is just my personal experience, but i thought it was at least worth sharing. i thought it's pretty much accepted that just because you have a degree in computer science doesn't mean you're an out of this world, or even competent programmer. why should there be a difference with software engineering?
Admin
Agreed. I am, to this day, quite guilty of being a real WTF coder when I code (which is rarely), but when I do code, I don't make mistakes THAT egregious. That was truly WTF coding - even if just by a student.
I have my BS in Comp. Eng. Tech. (from DeVry, no less - blech!), and I can recognize that code as truly horrible. If I can recognize that as shitty code, than just about anyone studying software design, coding, engineering should be able to as well. The only excuse for that crap is being given a test the very first day of your first year in college to see if you have any understanding at all of how to program. Seriously. An array for a 1 char value???!!! That's just retarded, and you learn what arrays are for in some of the earliest programming classes you have.
Sorry, but morons in college are still morons.
Admin
I dunno about you folks, but here's how I do the hex string to integer conversions:
int Hex2Dec(const std::string& HexString) { std::stringstream converter; converter << HexString; int value; converter >> std::hex >> value; return(value); }
Admin
c++.... where's the bug?
Admin
No, he is a student, Yes next week he could be out in the world, but he will be a junior now learning from those who have been in the field hopefully getting subjected to code reviews and refining his technique. Once he starts charging for his coding experience he is then fair game, at this time he gets nothing in return but ridicule.
There is no telling if he thinks his code is good enough to get paid for, and that makes all the difference in the world.
Admin
Meh, not a bug so much as intentional design. It's well documented that the input is supposed to be hex, so I didn't see the need to add exception handling to code that....doesn't handle exceptions... Worst case scenario it comes back saying the guy's password is invalid. I suppose I could write an error log to help analyze any problems...
Admin
The real WTF is that the function never ends because it lacks a closing brace, right?
Admin
Admin
Wrong way to think for a number of reasons IMO.
Documents lie all the time. Rely on them at your peril. How many times do you actually check your documentation is correct after you've fixed a bug?
I've done plenty of integration work where the customer swears blind that all their data confirms to a schema, dtd or some other agreed format and it never has.
If your unit of code is given an invalid input and it gives back a response that could be valid it is fundamentally broken. Your design contract does not include lying!
Basically validating the input costs you little and this good practise makes applications far more robust. It's simple to just throw an unchecked exception or whatever the C++ equivalent is, at least that way it will get logged somewhere even if that's on the error out. It's not simple and at least two orders of magnitude more expensive to debug something like this later.
/end sermon :)
Admin
Rather unfortunately, that generates no warning for invalid input (just returns 0, which is undefined as to it's correctness), which is likely why this code came about - part of the spec. for the assignment may have included alerting the user about input errors. Ah well, it beats using regexes in an undefined order into several variables, which may or may not initialize everything, and crashing without warning if an option isn't recognized (certain open source project).
Admin
The above should have been about strtol, but apparently the quote part of my message vanished into the ether...
Admin
While close, you off a bit - you don't check status of the stream to see if it failed. Take a look at boost::lexical_cast for a correct implementation using std streams.
Admin
Admin
Maybe not cool, but still valid.
Admin
Heh-- that reminds me of an answer that a friend of mine put on a midterm college computer science test.
We were studying for loops, and were asked to write a program (on paper) that would shift the characters around: The first line printed should print "A" through "Z", the next line "B" through "Z" with an "A" at the end, the next line "C" through "Z" with "AB" at the end, etc.
My friend couldn't grok how to do this (two nested FOR loops was used in my solution) during the test, so he wrote:
Print "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; Print "BCDEFGHIJKLMNOPQRSTUVWXYZA"; Print "CDEFGHIJKLMNOPQRSTUVWXYZAB"; Print "DEFGHIJKLMNOPQRSTUVWXYZABC";
(etc.)
He got half credit. The teacher put the clause "You must use loops" in subsequent tests.
Admin
No machine that code will ever run on will align the stack on less than two bytes, so his string is perfectly safe. Don't complain about working code! Go back to optimizing The Report, and give me some more face time in the office!
Admin
Well, that's just about the type of code i'd come up with if halfway through the project I'd get really pissed about the lecture or my team mates and wanted to "show them" (most likely since my assignment was so simple that it's boring me to even think about it), which is what the article title implies.
Somehow none of you believe that a student could write such code on purpose, It seems to me like a clear sign of "Dienst nach Vorschrift" (work-to-rule, although i'm not sure if that can have the exact same meaning: protest against some stupid rules and therefore doing exactly what's required but make it in any way possible different from what's expected)
Admin
PLEASE reinstate the No Student Code rule. I am sure we can pick any class (whether it is SE, or a grad class) and get a months worth of WTF's. Sure, many of these people will go on to write their own professional WTF's one day, but until they leave the Ivory Towers, leave them be.
I thought this site was supposed to concentrate on PRODUCTION WTF's. That is WTF's in shipping code, or in an internal process that is actually being used. Not in code someone wrote at 4 am for a 10am deadline that will never be used again.
I used to interview students, most of them CS students. I started off asking them to write strcmp. I probably interview a couple hundred students in the few years I interviewed. ONE came close to an optimal answer, and less than 10% wrote something that even worked. About a 1/4 didn't even quite grasp the concept. And these were people who were graduating and getting a CS degree! One guy claimed he just wrote an XML parser, and he didn't understand the concept of comparing two strings...
Admin
I agree its so fucking weak to use student code. Doesn't matter in what class it is, or how many years they have been in school. Shit we all started somewhere. The stuff I used to turn in back then was horrible. Some was a combination of not understanding, but frankly the other 50% was that I'd rather get laid, drunk or do anything other than sit and write code. I'd still rather get laid, drunk or anything else but sit and write code - but paying the bills is nice too
Th1s s1te suxors!
Admin
Admin
there's two ways you can do that (there is another one that uses c++ streams). there's
int XMLFeed::Reader::HexToDec(std::string* hex) { char *isvalid unsigned long l = strtoul(hex->c_str(),&isvalid,16); if (*isvalid == '\0'){ // something happened here and needs resolution } return l; }
or if you just want to waist time then:
int XMLFeed::Reader::HexToDec(std::string* hex) { const char* chHex = hex->c_str(); int acc = 0; for(; *chHex; chHex++){ if ((*chHex >= '0') && (*chHex <= '9')) acc += acc - '0'; else if ((*chHex >= 'A') && (*chHex <= 'F')) acc += 10 + (acc - 'A') else{ // something happened here and needs resolution break; } acc = acc << 4; } return acc; }
Admin
Rather than list mistakes I think it will be easiest to show an improved version and you can compare differences for yourself:
Note #1: it is exceptionally poor design for C++ code (as this clearly is due to the '::' operator) to return a pointer to memory allocated with 'malloc'. So I am providing a C version. For a C++ version you should allocate and return the memory differently. If you want a version that can be used in both languages, write the C version and declare the function with extern "C" in the C++ header.Note #2: Your version read off the end of the input when the length was odd. My version 'solves' this by ignoring the last character of an odd-length string. A better solution would involve specifying what to do with odd-length strings (left-pad with zero? right-fill with zero? right-fill with F?)
/* Caller must free return value */ char *Hex2Bin(const char* pHex, size_t nSize) { char *pBin = calloc( (nSize + 1) / 2 + 1 ); if ( pBin ) for( size_t i = 1; i < nSize; i+=2 ) pBin[i/2] = Hex2Char(pHex[i-1])*16 + Hex2Char(pHex[i]); return pBin; }Admin
Admin
Admin
Redux of WTFs (M = meta):
#M1: Ragging on student code. #M2: Writing this function, when XML accepts the un-converted input. #M3: Writing C-style code in a C++ function. #M4: Not using standard library functions that already do this. #M5: Function fails to accept lower-case hex.
#1: Pointer to std::string is a poor choice for function parameter. It should either be const reference to string, or pointer to const char.
#2: Declaring a local array of 1 element. (Almost always a WTF).
#3:
should be . The posted version causes a buffer overflow. Note the ugly syntax required due to the poor choice of function parameter type.#4: Doing a string comparison instead of a single character comparison.
#5: Behaviour on error condition leaves a bit to be desired. (At least he commented out the exit(1) !!)
#6: Causes undefined behaviour if pow(16, power) > MAX_INT.
#7: Using floating-point power function for integer exponentiation. The value could be wrong due to rounding errors (although I don't think this would happen on IEEE floating point).
#8: Using 'int' for the sum.
#9: (minor) 'power' could have been used as the loop counter, obviating the need for intHexLength;
Admin
I'd just like to say I'm sorry: I guess I didn't realize there was an issue with using student code, which is what the above is. Mea culpa.
(In other news, please stop sending me student code.)
Admin
I AM a CS student, and I knew better than to do that kind of crap since after about day 3 of my first freshman course.
Ignoring built in functions that do the same thing (and I am surprised that the student didn't use a built in functions, we are /great/ at that! :-D ), not knowing the difference between a char and a string is.... ick. Beyond stupid. I was given so many freshman assignments that consisted of character manipulations, and having to understand the base fact that at heart a character can be interpreted as an integer, that I have no idea how any student in any CS program anywhere manages to slip through without that understanding.
Admin
The return value of hex->substr(i,1) is a temporary std::string. Unless my understanding of C++ temporary lifetimes is incorrect (which it may very well be), I am pretty sure that the temporary std::string is destroyed before the call to strcpy().
Apparently he got "lucky" and the memory allocator didn't crap on the string when deallocating it, otherwise the code wouldn't have worked. (Or the buffer overflow might have been larger than it was.)
Admin
-tg
CAPTCHA Error: Word too long: diminutive
Admin
no I didn't even compile this because I could not be bothered and I thought you would understand what it was ment to do. but anyway just to make you happy I'll fix it for you.
int XMLFeed::Reader::HexToDec(std::string* hex) { char *isvalid unsigned long l = strtoul(hex->c_str(),&isvalid,16); if ((errno == EINVAL) || (errno == ERANGE)){ // something happened here and needs resolution // also it is an error if the string is empty as this // would mean that a required field is left blank. } return l; }and I suppose you didn't compile it either. that produces the following error:
Can I ask which char encodeing this won't work for? My code is still valid when used with EBCDIC, ASCII and uniCode. Also most compilers are that smart to detect such optimising.
Also to be pedantic, you failed to find 2 errors in my code. First is that it only excepts upper case chars. secondly. the result would also be wrong as it is shifted to the right by 4 bits will cause the function to produece invalid results.
int XMLFeed::Reader::HexToDec(std::string* hex) { const char* chHex = hex->c_str(); char tmp; int acc = 0; for(; *chHex; chHex++){ tmp = toupper(*chHex); acc = acc << 4; if ((tmp >= '0') && (tmp <= '9')) acc += acc - '0'; else if ((tmp >= 'A') && (tmp <= 'F')) acc += 10 + (acc - 'A') else{ // something happened here and needs resolution break; } } return acc; }Admin
Admin
In fact it persists until the end of the statement it's created in, so the code is OK. (If the creating statement is a loop condition, it persists until that loop condition has been evaluated).
Admin
Thank you for posting this!
The software engineering course at my school had virtually no prerequesites and so you got all kinds of grade levels in there, from sophomore to seniors. I believe there was even a graduate-level section merged in with ours.
The project was to write a conference reservation system based on PHP and mySQL. The few prerequesites for this class were classes taught in C++. The code I wrote was beyond horrible, namely because I didn't know much of any SQL and just did things the hard way by writing very simplistic queries and doing all the hard work in the PHP scripts. If I hadn't taken a perl class in the previous semester, I probably would have even struggled to learn PHP.
None of my queries were parameterized and so they were all vunerible to SQL injection attacks. I simply didn't know any better at the time! The sad part is that I was far and away the best programmer in my group. By the end of the semester, it was at least functional and our project was finished. I'm not proud of that code, but at least I know what I did wrong. Now that I'm 2 days away from earning my CS degree, I feel confident that I could rewrite that system securely in a matter of days if I tried.
Admin
In fact errno is not even set at all if no digits were found (eg. the string "foo"). strtoul does not set EINVAL either. (That is, the C standard does not specify that it does -- however, any implementation of a function can set any non-zero value of errno that it likes).
To detect conversion error you have to check the return value first. Only once you have determined the return value was ULONG_MAX would you check errno.
You're right that the 'isvalid' should be pointer to pointer to non-const char. However you have repeated your error of not finishing the declaration with a semicolon.
I don't know of any such encoding, but it could exist in theory. Probably shouldn't have mentioned it.Admin
You don't? That's a problem. IMHO, programming is not primarily about the tools used, but about thought, and awareness of context, and the use of pow in this context betrays a complete lack of that quality.
Look at the context. That 8-char string is broken down into two-char strings, right? Why the hell do you need the pow function to multiply the first digit of each pair by 16?!
Once again, I want to thank Jerry Ross [Lane Community College, Eugene, OR] my instructor in CS 161, for his wisdom in running us through Leslie Ann Robertson's "Simple Program Design" for six weeks before introducing any language specific material.
Think it through first--code later.
Admin
He wrote a HexToDec (which is a stupid name, by the way - "hexstring2num" would be better, or just "strtol" if you're into standard names), even though he's only using it for two-digit numbers. I don't think writing code slightly more general to be reused is a WTF.
Of course, the power thing is what a normal person would accomplish more quickly and without requiring an FPU by "sum = 16*sum + value" or even "sum = sum<<4 + value" at the same spot of the loop, shifting over a hexadigit each iteration rather than building it in place. (It turns out that positive integer powers are just repeated multiplications. Who knew?)
Admin
Endianness has nothing to do with it. The problem is with the char[1], and he'll get away with it as long as the local frame is padded, which has been safe to assume for quite a while -- maybe since the 90-s, I'm not sure (not that the poor sod would know anyway ;)
Admin
It's no longer safe to assume with GCC 4.
Admin
The more I read this site, the more I worry about the quality of coders on here and the kind of programming practices this site encourages.
THIS IS NOT A WTF!
The programmer does not use the sscanf function, which may have been optimal (probably because they don't know about it -- which is perfectly reasonable). Instead they craft their own implementation of the obvious algorithm to do it. The implementation is very clearly documented and easy to follow (if a bit long). There is a subtle bug with the: char strDigit[1]; //store 1 digit in that the null terminator from strcpy will overwrite something, but that is a common misstake and not an example of some crazy code that makes me want to throw up my hands and throttle the guy.
Anyway, I'm giving up on this site. Have fun!
Admin
The Real Portability Bug bug here is in
for (int i = 0; i < intHexLength; i++)
declaring the local i within the for statement is not supported in various flavors of gcc
Admin
while i was a horrible coder as a student, i would have never thought of doing it like this.
Admin
Here's one solution...
int strcmp( const char* a, const char* b ) { while( *a == *b && *a != 0 ) { a++; b++; } return static_cast<unsigned char>(*a) - static_cast<unsigned char>(*b); }What do you think? Flame/mock away...
Admin
I don't like the Horner scheme either. And what's std::string's operator[] all about anyway? This thing is so useless, you can't even use it to overflow a buffer.
Admin
Admin
Admin