nice_num, mean_programmer

  • IronMensan 2008-11-28 09:08
    I've written a function that does this before. It has to process one digit at a time instead of going 1,000 at a time, otherwise you end up with "1,23" instead of "1,023"
  • Jay 2008-11-28 09:30
    I don't know... What's so horrible about this?

    This way, you can get right-aligned output quite neatly. Add sufficient amount of spaces in front of first digit (or sign) and that's it, just make sure you keep it to the allocated buffer. There's virtually no overhead compared to string concatenation.

    And dividing by 1000... You have to divide by 10 anyway to produce the individual digits. Additional divisions by 1000 would be reduntant and slower.

    It's optimized for speed, obviously, and yet, at least to me, program flow was easy to follow. So, where's the WTF?
  • Adam 2008-11-28 09:30
    Not sure this is a WTF really

    it work and doesn't seem too ugly...
  • DC 2008-11-28 09:33
    I would think you could use some string formatting to pad each section with 0s. But in any case that's just an efficiency question.

    There's nothing in this function that prevents the pointer from going lower than its original value. Also, it uses what is apparently a global array as the actual destination, and just assumes the size is at least 20. So, there is a possibility of overwriting memory on both ends. (Unless a long value can't get big enough that it would fill more than 20 digits, it's too early for me to think about that though.)
  • IronMensan 2008-11-28 09:33
    And what's "goofy" about modulus math?

    I know of two ways to implement this function. One is like the code presented here, the other way is to use sprintf and adjust the results. You still have to deal with edge cases there to make sure you don't end up with "-,123"

    Is it just me, or is the real WTF why this got posted as a CodeSOD?
  • Shinhan 2008-11-28 09:33
    C doesn't have number_format() ?
  • helix 2008-11-28 09:35
    looks standard fare for an embedded firmware engineer.
  • Adam H. 2008-11-28 09:35
    Um, this function is fairly clean, blazing fast, and reasonably easy to understand for a competent C programmer. It's not thread-safe in the least, and it has buffer-overflow issues if you try to port the code straight into a 64-bit environment (maximum string length of 20), but calling this a "WTF" is a stretch. Other than fixing these two problems, I would be hard pressed to write a more elegant method for solving this problem.

    "I don't know the language very well and therefore code examples written in it are hard to read" doth not a WTF make.
  • GettinSadda 2008-11-28 09:35
    Seems OK to me.

    Variable names could be a little better perhaps, and you should really comment something like this, but I don't see what's wrong with it.

    One improvement that I would probably make is to make the loop infinite with an if(!n) break before adding the comma as this means that you don't have to remove it afterwards.
  • Dissident 2008-11-28 09:38
    Apart from global variable and possible buffer overflow (already pointed out), this looks pretty ok.

    Assuming there are no library functions and you really have to code a function to format text, I challenge the OP to come up with something better, more elegant and/or more efficient.
  • mscha 2008-11-28 09:39
    A function that has variables?? Shocking! :-P

    - Michael
  • MAG 2008-11-28 09:42
    The real WTF is people thinking that having no comments, pointless and not so helpful variable names and a stupid function name is not a WTF.

    Also, isn't there any number format C function??
  • welshie1988 2008-11-28 09:49
    Looks fine to me. Give it a long and you get out a nicely formatted user-friendly string in a buffer. Apart from the concurrency/overflow issues mentioned, this is fine.
  • summerian 2008-11-28 09:50
    IronMensan:
    I've written a function that does this before. It has to process one digit at a time instead of going 1,000 at a time, otherwise you end up with "1,23" instead of "1,023"


    No, it does not have to process one digit at a time. You just check if the remainder is below 10 or 100 to add leading "00" or "0".

    But it guess it's the same either way.
  • Pascal Cuoq 2008-11-28 09:55
    Apart from the comment saying that this code will break when 'long' is strictly more than 64 bits and a meaningful function name, I do not see either what the submitter is complaining about. These is nothing "goofy" about dividing by 10 and taking the modulo to get to the digits.

    Pascal

    Oh, one more thing: the function breaks for MIN_LONG because the instruction
    n = -n; overflows in this one and only case. Since it is only one case and
    if the size of long is know, the simplest fix is to have a static literal string
    for MIN_LONG and to test for this value.
    Somehow, I think that this is not the reason the submitter sent this function.

    The value analysis in Frama-C (http://frama-c.cea.fr) finds this kind of overflows,
    but then it emits false alarms too.

  • Geoff 2008-11-28 09:56
    I'll give a WTF for using what appears to be a static buffer and assuming a long will fit a char[20]. Other than that, it looks reasonable. Hopefully the compiler is smart enough notice it can accomplish the mod and the divide with one instruction on most processors.
  • Pascal Cuoq 2008-11-28 09:59
    Contrary to what I just said, there is indeed a risk of buffer overflow even with just 64 bits for longs. I forgot to count the minus sign and all the commas.

    Pascal

  • IronMensan 2008-11-28 10:04
    True, but that introduces another edge case to avoid "001,023" That leads to more instructions, including branches. Granted we are only talking about a few cycles difference.
  • Oxyd 2008-11-28 10:10
    It's C (not C++) -- this just is what the language looks like. Aside from "minor" issues with static-length buffer and stuff, I don't think it could be made any better while still keeping it C.
  • Darryl 2008-11-28 10:11
    There are a couple of things here that would raise a flag if I was doing a formal code review (like the handling of edge cases, and the use of a global buffer that may not be big enough), but the approach as a whole is sound. It's clever, it's efficient, and honestly it should be pretty straightforward to understand for any C programmer. I knew what it was doing, and how it was doing it, after one read, top to bottom, without having to backtrack. To me, that's decent code.
  • bitpirate 2008-11-28 10:13
    The real WTF is assuming that all locales use "," for their thousands separator. "1.000.000" is common for a million in Europe.
  • Death 2008-11-28 10:17
    IF this is for an embedded/micro controller application(where using a library may not be worth it), its pretty OK, else... Its not nice to read, not entirely safe for all cases and in bad need of comments. Not sure if that's enough to say WTF, tho...
  • DaStoned 2008-11-28 10:18
    helix:
    looks standard fare for an embedded firmware engineer.


    Hell yes, I've written dozens of similar functions. When string conversion/formatting is not available (due to memory constraints) it's the only way to go. No WTF here.

    On the other hand, I have also spent many long hours debugging such functions. I use printf and friends instead of home-cooked conversion whenever possible.
  • Lamah 2008-11-28 10:23
    Submitter does not know any C if they think that is a WTF.
  • Mcoder 2008-11-28 10:24
    First, the code is quite clean and fast. Starting at the end is clever, but not so clever as to make it unmaintanable (what is great), and there is no need for coments, the code is obvious, it just need a better name, so people don't have to read it at all.

    Now, there is no buffer overflow for a tipical 32 bits long. People around here thinking that long means a 64 bit number is, well, weard. Ok, there is no garantee that a long will have only 32 bits, but that was the standardized size. There is also no garantee that a long will have 64 bits, just 32.

    If there is a WTF at all, it is that the developer didn't use some standard library. Now, if that is embebed code, he has plenty of reasons to both not use standard libs nor expect a long that has more than 32 bits.
  • Asiago Chow 2008-11-28 10:25
    Maybe the buffer is actually larger than 20. That's a fairly common kludge. It would've been nice to see a "//20 is to keep the results inside a char[27]"

    The use of an external buffer is always a pain but I'd rather see that than a pointer to a local scope variable -- that's a real nightmare waiting to bite.

    As for localization... many applications simply don't need it.
  • GettinSadda 2008-11-28 10:26
    DaStoned:
    helix:
    looks standard fare for an embedded firmware engineer.


    Hell yes, I've written dozens of similar functions. When string conversion/formatting is not available (due to memory constraints) it's the only way to go. No WTF here.

    On the other hand, I have also spent many long hours debugging such functions. I use printf and friends instead of home-cooked conversion whenever possible.


    Remind me again just how you get commas in numbers using standard printf?
  • Asiago Chow 2008-11-28 10:29
    Err, scratch my buffsize being different than buffer size comment... just waking up.
  • Kokuma 2008-11-28 10:30
    I'm quite surprised by the line "Yeah, it takes a few minutes to figure out." It seems rather straigthforward to me. The size of the buffer (20) might prove a little short (because he adds ',' in the string), there should be a better solution to fetch the thousands separator, maybe there's some builtin format in sprintf that does this, or a first sprintf might prove more effecient, and then add the separators, I don't know, but that's some code I wouldn't be surprised to find in a commercial product... Frankly, unless it proves to be a perfomance bottleneck, I'd let such code in the code base (well, after renaming it, perhaps... something more along the lines of "format_long" or whatever)
  • NameNotFoundException 2008-11-28 10:37
    Wrong brace style! WTFFFFF!
  • tlg 2008-11-28 10:38
    I'm going to have to agree with the masses - seems fair enough to me.
  • vicz 2008-11-28 10:39
    printf("%'d",number);

    * locale determines separator character
    * may nt be supported everywhere
  • kennytm 2008-11-28 10:43
    Programmer mean_programmer (Programmer programmer_list[], int count) {
    Programmer all_programmers;
    for (int i = 0; i < count; ++ i)
    all_programmers += programmer_list[i];
    return all_programmers/count;
    }
  • Clarence Odbody 2008-11-28 10:45
    Other than the mysterious pre-allocated buffer and the assumption of maximum size this function is very good. In fact I've saved it as useful snippet.
  • JPhi 2008-11-28 10:46
    GettinSadda:


    Remind me again just how you get commas in numbers using standard printf?


    You use sprintf to get the number into a string, and then you simply add a comma every 3 spaces in some kind of loop... It takes all the "math" out of the equation and turns the problem into a simple string manipulation.


    And for all of you asking "isn't there a library function to do that", the answer is NO. The C language is about getting stuff done -- it's not about reading a book or searhing on the internet for 10 minutes to find a library somewhere that does what you want to do. If I was faced with the comma problem, I would have done exactly what the OP(rogrammer) did.

    And for everyone else talking about buffer overflows and global variables, etc., it might just be me, but these "advanced" problems DO NOT constitute a WTF. Chances are very good that this function would be used to convert a known range of numbers and none of your theoretical problems would ever come up. Is it the best coding practices? NO! It's not even very maintainable because of the globals and the "magic numbers", but for all we know, this snippet was from a small, simple program that was done quickly an efficiently.

    To me, a WTF is something that simply makes no sense at all -- not trivial technical mistakes. We don't want this to be like that English nazi site with 1000's of posts making fun of signs that use "your" instead of "you're".

    So, thumbs down to the WTF, and that's too bad because there have been some good ones lately!
  • Paolo G 2008-11-28 10:46
    bitpirate:
    The real WTF is assuming that all locales use "," for their thousands separator. "1.000.000" is common for a million in Europe.


    Yes, dot is the standard in continental Europe (which uses a comma instead of a decimal point), but the comma is used in the UK.
  • Paolo G 2008-11-28 10:48
    kennytm:
    Programmer mean_programmer (Programmer programmer_list[], int count) {
    Programmer all_programmers;
    for (int i = 0; i < count; ++ i)
    all_programmers += programmer_list[i];
    return all_programmers/count;
    }


    We'll assume that the default constructor for Programmer initialises it to 0, but other than that, you forgot to handle the case when count equals 0.
  • Gieron 2008-11-28 10:53
    Paolo G:
    bitpirate:
    The real WTF is assuming that all locales use "," for their thousands separator. "1.000.000" is common for a million in Europe.

    Yes, dot is the standard in continental Europe (which uses a comma instead of a decimal point), but the comma is used in the UK.

    In Sweden we use spaces: 1 000 000
  • Evo 2008-11-28 11:02
    I agree, not much of a wtf here. In C++ (although I don't see anything indicating this is C++ and not C), however, this is a better way to do it portably:

    std::stringstream ss;
    std::string formatted;
    ss.imbue(std::locale(""));
    ss << num;
    formatted = ss.str();

  • Darren 2008-11-28 11:05
    I'm assuming it's on some embedded platform with limited library support.

    Anyway, looks completely fine to me, and it really only takes a couple of seconds to figure out what it's trying to do.

    The function name is a little silly, maybe nice_number_format() instead.
  • Chip 2008-11-28 11:09
    mscha:
    A function that has variables?? Shocking! :-P

    - Michael


    That right there is TRWTF. =p
  • Robajob 2008-11-28 11:14
    JPhi:
    We don't want this to be like that English nazi site with 1000's of posts making fun of signs that use "your" instead of "you're".


    I do.
  • Osno 2008-11-28 11:15
    I got the meaning of the function on my first read, and I'm not even a C programmer. Ergo, comments would be redundant here.

    I'd change the name (a little) to pretty_print, or something more meaningful, and solve the few minor defects the function has, but that's it.

    I think we're way past the "uses foo as a name function" WTF. If we're back to posting stuff because of bad naming conventions, plus the ban on TopCoder, this site is doomed.
  • shepd 2008-11-28 11:23
    Commas, periods, for thousands separators. How about the Canadian way, with spaces? All Canadian schoolchildren know about this. Although, once we grow up, we drop the pretentiousness and just do what the USians do.
  • Alex 2008-11-28 11:32
    helix:
    looks standard fare for an embedded firmware engineer.

    I bet this is the case. You don't always have all C library functions available; and you want everything to be pretty fast; and you can allow yourself some shortcuts (no overflow check), because you know the hardware.
  • Kerio 2008-11-28 11:32
    the real wtf is this codesod

    captcha:tristique - it's how i'm feeling looking at this snippet
  • Moo 2008-11-28 11:32
    I would really like to know what the wtf is. What exactly is the problem with this code? Comments? Comments are useless, given how straightforward the code is, and would detract from its clarity.

    The largest possible string for a given input is "-2,147,483,648\0", which is 15 characters, so there is no buffer overflow. The function is static so is only used in one file. All that's missing is the external declaration

    static char prtbuf[20];

    and an example of usage:

    printf("The number is %s\n", nice_num(foo));

    which is the equivalent of

    printf("The number is %i\n", foo);

    except that it has thousand separators.

    Given the local scope, the obviousness of its implementation, and the obviousness of its usage, what's wrong with the function name?

    And how exactly are you supposed to convert binary to a decimal string without dividing by 10?
  • Evo 2008-11-28 11:39
    Moo:
    The largest possible string for a given input is "-2,147,483,648\0", which is 15 characters, so there is no buffer overflow.


    I agree with you in everything but this sentence. The size of long may be anything, as long as it is at least as long as an integer. It may be several megabytes for all you'd know. In practice, it will be either 4 or 8 bytes. On my 64 bits processor, it's 8 bytes, which makes the largest possible string:
    "-2,305,843,009,213,693,952\0"
    which is 27 characters long.
  • ZPedro 2008-11-28 11:42
    I'm going to have to join the chorus here: I don't see much of a WTF; especially the "few minutes to figure out" argument doesn't hold, for me "I came, I saw, I understood"; granted, I know C better than my own mother by now, but even then it's pretty understandable. It might be a bit too clever by half with predecrements, and I'd at least put the global string buffer size in a #define and of course give the function a better name, but otherwise I'd just comment its limitations (restricted to 32 bit, not thread-safe, locale (if applicable for context)) and move on. It even gets the '0' edge case right!

    I guess everything depends on context, and if this was used in a web app it may be a WTF, but the mere fact it's written in raw ANSI C suggests a context with few luxuries in which this code makes perfect sense.

    In short, nice_function, mean_Vinson.
  • Bob 2008-11-28 11:43
    [quote user="JPhi"][quote user="GettinSadda"]
    To me, a WTF is something that simply makes no sense at all -- not trivial technical mistakes. We don't want this to be like that English nazi site with 1000's of posts making fun of signs that use "your" instead of "you're".[/quote]

    When you say "We", I think you mean "I" and when you say "1000's", I think you mean "1000s".

    Normally I ignore any post that appears to be written by an illiterate child but the rest of yours was okay.
  • Dr Leather 2008-11-28 11:46
    I came across a similar bit of Java code in our project. Feeling all clever, I re-wrote the thing using Java's utility number format, then benchmarked a few million iterations so I could feel even more clever about improving both the codebase and the performance. Turned out that clunky old pile of WTF was actually 2 orders of magnitude faster than a one-liner using the standard number format libs.
    I knew the number format classes weren't exactly super fast, but that was an eye-opener.
    I learned a lesson, but I'm not sure it was a good one...
  • Magnus 2008-11-28 11:47
    bitpirate:
    The real WTF is assuming that all locales use "," for their thousands separator. "1.000.000" is common for a million in Europe.


    So is 1 000 000.
  • eb0ny 2008-11-28 11:52
    sprintf(buffer, "%d.%d", n/100, abs(n)%100);

    Enough said.
  • GettinSadda 2008-11-28 11:57
    eb0ny:
    sprintf(buffer, "%d.%d", n/100, abs(n)%100);

    Enough said.
    FAIL
  • DaveAronson 2008-11-28 12:10
    MAG:
    The real WTF is people thinking that having no comments, pointless and not so helpful variable names and a stupid function name is not a WTF.

    You clearly have not "inherited" much code. For better or horribly worse, that is SOP, not WTF.

    That said, though, it took far less than "a few minutes" for me to see what this code was doing, and exactly how. More like a few seconds of looking at the first several lines for a general clue, the next few to be a bit more specific, and the rest is just confirmation and details. It has a few issues, like the potential buffer underflow if ported to 64-bit or if the external buffer isn't big enough, and yes the names could be clearer, and some of the predecrements aren't clear if you don't already grasp what it's doing, but other than that, this is perfectly good fast clean C.

    TRWTF is people who can't understand this code pretty much instantly, having jobs in programming.
  • Daniel 2008-11-28 12:10
    If it takes you a few minutes to understand it, go chose another profession. This function is pretty obvious and, in fact, it's a reasonable implementation, assuming the paradigm it was obviously written in.

    One might look down on the global variable prtbuffer, but such things weren't always thought of as bad.
  • pm 2008-11-28 12:12
    Asiago Chow:

    As for localization... many applications simply don't need it.


    If it is a snippet inside of a small application, that might work, but still I don't like that. I guess I've just had to deal too much with problems because so many people skip localization issues. Here in Finland the locale is spaces between thousands etc. and comma as decimal separator. Of course that does not mean that all input will come in as such: the thousands separators may be missing, users may write decimal separators in GUI as commas OR as periods, when parsing output by another program or service you can never know for sure which locale it has used. So, even setting a locale or parsing with a certain locale is not a solution, in real life you need to put strings first through some kind of wrappers or filters.

    I've worked mostly with Java enterprise stuff lately, which includes lots of moving data as xml or something so this is quite a common issue for me. As someone said that branch of IT often means using third party libraries and tools and too many of them just ignore the issue and assume that the whole world uses just one kind of formatting. It is quite annoying that such a basic thing like parsing a numeric value from a string must always be paranoidly checked and watched.

    And of course it is not just numbers, Christmas Eve is 24.12. here, not 12/24, but the decimal separator issue is a clear winner here.

    For not to be completely off topic, I also vote this to not to be real WTF. After rotting my brain with that Java EE stuff and not using pointers actively for almost a decade or so that seemed quite trivial. Maybe pointer arithmetics are like what they say about driving a bicycle, once you get it ...
  • Alex 2008-11-28 12:16
    Interesting, been to Sweden a couple of times, and hadn't noticed that... (which doesn't paint me in a good colour, I know ;-) Thanks for pointing it out!
  • Daniel 2008-11-28 12:20
    DC:
    There's nothing in this function that prevents the pointer from going lower than its original value. Also, it uses what is apparently a global array as the actual destination, and just assumes the size is at least 20. So, there is a possibility of overwriting memory on both ends. (Unless a long value can't get big enough that it would fill more than 20 digits, it's too early for me to think about that though.


    It looks like old code. So you can assume long is 32 bits, and, thus, 20 is long enough. A global "print buffer" array was pretty common, and, in fact, wasn't really any burden on the environments we had.

    It's not 2008's webservicy OO code, of course, but you can complain about that to me when you do an accounting program in 4 KB RAM.

  • mudak 2008-11-28 12:23
    In binary you just have to know is it little-endian or big-endian. Why bother with text formats, locales and stuff?
  • NiceWTF 2008-11-28 12:26
    There are some (small?) WTF's in here:

    - The function name is quite non-descriptive
    - The function uses a global variable, likely there is no good reason for this (unless this is used in some embedded device with very limited memory)
    - It assumes (probably rightly) that the global buffer is sufficiently large
    - It assumes that the buffer is filled with spaces, or at least not with random garbage.

    However, it seems the real WTF is the submitter, in this case:


    It works backwards, starting with the string terminator, then setting the one, ten, hundred, etc. digits using goofy modulus math.

    Doing that completely makes sense. Also, there is nothing goofy about using modulus math, especially not to do something like this. In fact, short of using built-in library functions (not always available on embedded devices), I would hardly have a clue how to implement this *without* using modulus math (although I'm sure it can be done in some convoluted way).

    The function divides by ten with each iteration (rather than 1,000 for some reason)

    "some reason" being that otherwise it would not fucking work, at all.

    And if it was negative to begin with, it mashes a minus sign back on to the string before returning it.

    Yup, seems fine to me. The remaining problem is that it does not fill the remaining buffer size with spaces, i.e. it assumes the buffer is filled with spaces beforehand, which is a bit awkward.
  • sadwings 2008-11-28 12:27
    I like the code and learned a bit by reading and playing with it.

    The only thing I'd do different is this, but maybe there's a reason not to.

    // only put a comma if there are more bytes in n
    if ( ( ( ++d % 3 ) == 0 ) && n ) *--buffer = ',';


  • thosrtanner 2008-11-28 12:34
    Why would it need to fill the buffer with spaces? It returns a pointer to the start of the string it has generated, i.e. the '-' sign or the first digit.
  • cheers 2008-11-28 12:34
    Btw it returns the pointer to the last position it wrote to so it has no requirement that the buffer is filled with spaces or anything.
  • cheers 2008-11-28 12:39
    sadwings: That'll require a few more operations than just disregarding a surplus comma at the end. In fact, the modulus on d is going to be a lot slower than comparing d with 0 and resetting it to 3. These are very low-level differences with extremely small gains in speed but for an embedded device with a cruddy processor it might make a bit of a difference. Whoever wrote the code was obviously aware of this.
  • Daniel 2008-11-28 12:42
    Evo:
    Moo:
    The largest possible string for a given input is "-2,147,483,648\0", which is 15 characters, so there is no buffer overflow.

    I agree with you in everything but this sentence. The size of long may be anything, as long as it is at least as long as an integer.


    TODAY. Once, it was guaranteed to be 32 bits.

  • WhiskeyJack 2008-11-28 12:45
    Yup, gonna sing with the chorus today. I saw the function name, saw it looped around the digits of a number, saw the insertion of commas, and immediately knew what this function was doing. Spent a few more seconds looking for anything WTF-worthy, and didn't find anything.

    Sorry Alex, not today.
  • Daniel 2008-11-28 12:45
    NiceWTF:
    - It assumes that the buffer is filled with spaces, or at least not with random garbage.


    You did not read the code. It returns the pointer at the beginning of the number, NOT the beginning of the buffer.

  • lagfest 2008-11-28 12:50
    NiceWTF:
    There are some (small?) WTF's in here:

    ...

    - It assumes that the buffer is filled with spaces, or at least not with random garbage.

    Wrong. It returns a pointer to the first character of the number, not the start of the buffer.
  • Asiago Chow 2008-11-28 12:51
    pm:
    Asiago Chow:

    As for localization... many applications simply don't need it.


    If it is a snippet inside of a small application, that might work, but still I don't like that. I guess I've just had to deal too much with problems because so many people skip localization issues. ... And of course it is not just numbers, Christmas Eve is 24.12. here, not 12/24, but the decimal separator issue is a clear winner here.


    I won't argue with you. I deal with a lot of financial software and converting strings to numbers which means I have seen a LOT of ways of representing -2910.74 such as "(291074)", "2,910.74-", "2910.74 (overdrawn)", "-2911", "10000291174", etc., along with a bunch of date formats I won't get into. That's how it the numbers are shown on screen for humans to read. Bizarre just starts the conversation.

    OTOH: A financial app written for the US market is likely closely tied to US tax code and regulation anyway... nobody is going to pick up a copy while they are on holiday and install it when they get home...or, if they do, the localization oddities won't be the biggest problem. :)

    Not everything is innately international.
  • Moo 2008-11-28 13:06
    Evo:
    Moo:
    The largest possible string for a given input is "-2,147,483,648\0", which is 15 characters, so there is no buffer overflow.


    I agree with you in everything but this sentence. The size of long may be anything, as long as it is at least as long as an integer. It may be several megabytes for all you'd know. In practice, it will be either 4 or 8 bytes. On my 64 bits processor, it's 8 bytes, which makes the largest possible string:
    "-2,305,843,009,213,693,952\0"
    which is 27 characters long.


    Sure, it's Implementation Defined, and you can find out what your implementation defines it as by reading LONG_MAX and LONG_MIN from <limits.h>. Does that make it a WTF?

    Assuming that the original programmer read the documentation for his implementation and was aware that it was non-portable, I still don't see a WTF. Ok, so maybe he should assert(sizeof(long) == 4).
  • operagost 2008-11-28 13:08
    shepd:
    Commas, periods, for thousands separators. How about the Canadian way, with spaces? All Canadian schoolchildren know about this. Although, once we grow up, we drop the pretentiousness and just do what the USians do.

    What's a USian?
  • Michael 2008-11-28 13:10



    Program in C,
    Program in C,
    Nothing is better
    than that single letter,
    take it from me!
    Interpreters take your speed away
    and python is totally gay,
    while we're devoting
    our time to floating
    -point numbers in C
  • operagost 2008-11-28 13:11
    Magnus:
    bitpirate:
    The real WTF is assuming that all locales use "," for their thousands separator. "1.000.000" is common for a million in Europe.


    So is 1 000 000.

    The real WTF is assuming that this code is designed to be used in multiple locales. "Hello world" would start at a million lines of code if it had to be designed with every possible locale, architecture, operating system, economy, political affiliation, religion, and sexual orientation taken into account.
  • neophrene 2008-11-28 13:18
    There is nothing wrong with this function. A good programmer understand it in far less than a minute. Of course it would be better to have a comment on top of it, but that's all. If your are not a good programmer (or if you are just a Java XML drone) you might of course want to fill it with storytelling comments.

    A good programmer also understand why it has been written like that.

    Indeed a good programmer already have written lots of functions like that.

    Please come back to us when you have enough programming experience, in various fields.
  • zaphod 2008-11-28 13:28
    operagost:
    "Hello world" would start at a million lines of code if it had to be designed with every possible locale, architecture, operating system, economy, political affiliation, religion, and sexual orientation taken into account.
    #if ORIENTATION==STRAIGHT
    #define MESSAGE "Hello world"
    #else
    #define MESSAGE "Hello sailor"
    #endif
  • Nazca 2008-11-28 13:32
    Have we really sunks this low?

    Okie, repeat and summerise...

    In a lot of places, there is no justifiable reason for calling out to sprintf for this.
    It's very clear what it does.

    With the exception of the mentioned overflow, it's good.

    Comments: If you need comments to read something like this, you probably need more coffee or a book on C.
    Thread safetfy: Not all programs are threadded. If this is embedded code, or a heavy linear cruncher, threading could well be counter productive.
    Going in tens/Modulus use: There's no other way to do it. It's very simple char offset printing.
    Variable names: um, why? digits_left_til_comma wouldn't gain you much over d in this case.
    Overflow possiblity: Cash register? If it's outputting to a fixed size display, or outputting a limited set of data, it probably has bounds checks elsewhere·
    Spaces to the front of the buffer: why? that assumes the author wanted to pad.

    A couple of special mentions ...

    GettinSadda:
    One improvement that I would probably make is to make the loop infinite with an if(!n) break before adding the comma as this means that you don't have to remove it afterwards.


    Which in most numbers is less efficient, since you're doing an extra comparison every loop, instead of a single comparison and increment at the end.

    JPhi:
    You use sprintf to get the number into a string, and then you simply add a comma every 3 spaces in some kind of loop... It takes all the "math" out of the equation and turns the problem into a simple string manipulation.


    Yeah...
    sprintf to one buffer and loop the array assigning to a second buffer which is returned.
    So you have all the overhead of sprintf and then do a manip loop anyway, with the added bonus of having to pull the data from a buffer instead of compute it.

    Talk about cutting your nose off to spite your face lol.

    And for all of you asking "isn't there a library function to do that", the answer is NO. The C language is about getting stuff done -- it's not about reading a book or searhing on the internet for 10 minutes to find a library somewhere that does what you want to do. If I was faced with the comma problem, I would have done exactly what the OP(rogrammer) did.


    As has been mentioned, there are library functions for this. And considering the first paragraph of your post, I find the comment of C being about getting stuff done to be funny.
    Yes, I know you wouldn't use sprintf, but it's still funny :p

    It's not even very maintainable because of the globals and the "magic numbers", but for all we know, this snippet was from a small, simple program that was done quickly an efficiently.

    The global may have been defined a couple of lines before the function, or may be shared between several such formatters. That doesn't make in unmaintainable.
    Neither do magic numbers... the only one I see is the buffer size, and we don't even know for sure that the buffer isn't a 4k buffer shared between formatters. :p


    Now goodness help us if someone had done int len = (buffer - prtbuf) - 20;

    Or... for (i=in,o=out;*o;*(i++)=*(o++));

    Heck... I suppose you'll find this a wtf:

    const int ALLOW[256] =
    
    {
    /* 0, 1, 2, 3, 4, 5, 6, 7, 8 9, A, B, C, D, E, F */
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*0x*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*1x*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*2x*/
    -747,-747,-747,-747,-747,-747,-747,-747,-747,-747,0x00,0x00,0x00,0x00,0x00,0x00, /*3x*/ /* 0 = 0x30, 9 = 0x39 */
    0x00,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747, /*4x*/ /* A = 0x41 */
    -747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,0x00,0x00,0x00,0x00,0x00, /*5x*/ /* Z = 0x5a */
    0x00,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747, /*6x*/ /* a = 0x61 */
    -747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,0x00,0x00,0x00,0x00,0x00, /*7x*/ /* z = 0x7a */
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*8x*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*9x*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*Ax*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*Bx*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*Cx*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*Dx*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*Ex*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 /*Fx*/
    };
    char chk_validity(char *s)
    {
    static char *c;
    for (c=s;*c;c++) if (ALLOW[c] != 0) return (char)0;
    return (char)1;
    }


    Addendum (2008-11-28 13:52):
    for (c=s;*c;c++) if (ALLOW[c] == 0) return (char)0;

    *coughs, looks sheepish, and hopes no one replied yet*
  • Eric 2008-11-28 13:43
    OK, in the vein of minor improvements (other than maintainability, like variable names), I think this would improve the speed by a couple of clicks:
    static char *nice_num(long n)
    
    {
    int neg = 0, d = 3;
    char *buffer = prtbuf;
    int bufsize = 20;

    if (n < 0)
    {
    neg = 1;
    n = -n;
    }
    buffer += bufsize;
    *--buffer = '\0';

    do
    {
    if (d == 0)
    {
    d = 3;
    *--buffer = ',';
    }
    *--buffer = '0' + (n % 10);
    n /= 10;
    d--;
    }
    while (n);

    if (neg) *--buffer = '-';
    return buffer;
    }

    It takes just as long while looping through n, but because it tries to add a comma at the beginning of the loop and then decrements d at the end, it won't add any superfluous commas at the beginning of the string. Thus, we save time by not adding and then having to check and remove it.
  • GettinSadda 2008-11-28 13:56
    Nazca:
    GettinSadda:
    One improvement that I would probably make is to make the loop infinite with an if(!n) break before adding the comma as this means that you don't have to remove it afterwards.


    Which in most numbers is less efficient, since you're doing an extra comparison every loop, instead of a single comparison and increment at the end.
    Actually if you look at the original code, the only changes are adding a conditional jump after the division by 10 (which will be very low cost as the test for zero will have probably been calculated by the division) and change the conditional jump at the end of the loop to an unconditional (this conditional is likely to be a more expensive than above as the manipulating of d means that you will now need to re-test the value of n). The upshot is that most systems should be faster doing the test in the middle.
  • Marcos 2008-11-28 14:00
    Damn, it looks like a perfectly good function to me... I hope I don't end up there some day.
  • m 2008-11-28 14:06
    MAG:
    The real WTF is people thinking that having no comments, pointless and not so helpful variable names and a stupid function name is not a WTF.

    Also, isn't there any number format C function??


    It's a very self-descriptive function name, perfectly understandable variable names, totally obvious control flow, and only idiots who having learn PHP a week ago think they know about programming think comments in a function like that are anything but an obstruction that makes it harder to understand. Not to mention that what it does is on the level of the second hour of programming 101 class. Too bad Jake apparently hasn't heard about modulo division or positional number systems.

    That's a perfectly fine function, and guess what, sometimes there aren't standard functions available. My bet (especially given the apparent threading/overflow thing pointed out before) is that it's from embedded code, where it's totally okay to write unsafe code to save a few bytes.
  • iMalc 2008-11-28 14:12
    Struggling to find any WTFs?

    In today's world where multithreaded apps are everywhere, this function really sucks. It puts the resulting string in a global buffer! Call that from two different threads at around the same time and at best one of those threads will see the other thread's output instead of its own.

    Or how about the fact that it fails for the most negative number a long can represent (0x80000000), because in that case -n equals n in two's complement.

    Other than those major problems and the lack of comments and short variable names (which lets be honest many of us would do the same), it seems usable in limited circumstances.
  • me 2008-11-28 14:28
    When I was 12 years old, I wrote similar code in CBM BASIC 2 for some task. However, IIRC, the code was much simpler than that.

    That were times...
  • Mark Bowytz 2008-11-28 14:31
    Eric:
    OK, in the vein of minor improvements (other than maintainability, like variable names), I think this would improve the speed by a couple of clicks:
    ...snipped Eric's code...

    It takes just as long while looping through n, but because it tries to add a comma at the beginning of the loop and then decrements d at the end, it won't add any superfluous commas at the beginning of the string. Thus, we save time by not adding and then having to check and remove it.

    Strangely enough, I don't think there's too much you can do to improve the original code. I ran the original, vs Eric's vs my contrib to this thread:

    char *my_nice_num(unsigned long n)
    
    {
    char *p = prtbuf;
    int i = 0;

    *p = '\0';

    do {
    if(i%3 == 0 && i != 0)
    *--p = ',';
    *--p = '0' + n % 10;
    n /= 10;
    i++;
    } while(n != 0);

    return p;
    }

    In the end, after formatting 0 -> 5,000,000 (did that formatting myself, no program needed!) they all took about the same amount of time (under 20 seconds each set...I have a lousy pc)

    In defense of this SOD being a WTF - I write piles and piles of C code at the day job. Midday, I could figure this out no problem. If I had to figure it out during a middle-of-the-night production problem, no chance. In other words, TRWTF is that C stinks at re-formatting numeric output.
  • Johnny Canuck 2008-11-28 14:34
    iMalc:
    Struggling to find any WTFs? In today's world where multithreaded apps are everywhere, this function really sucks.


    Yeah, and in today's world, Babe Ruth wouldn't make it out of minor ball, Joe Namath wouldn't last 5 minutes and Wilt Chamberlain would've been luck to sleep with 50 women. (Although, in today's world, Bobby Orr would've been able to have knee surgery and likely play another 5-7 years...)

    My point being that this looks like code from before. Not worried about 64 bit or multi-threading. It does what it's supposed to do quickly and efficiently. In it's world, this code is WTF-free and I'm kinda surprised to see it posted here.
  • TopCod3rsBottom 2008-11-28 14:42
    Mcoder:
    Now, there is no buffer overflow for a typical 32 bits long. People around here thinking that long means a 64 bit number is, well, weird. Ok, there is no garantee that a long will have only 32 bits, but that was the standardized size. There is also no garantee that a long will have 64 bits, just 32.

    In this century, 32 bit longs should be assumed to be exceptions to the rule, and 64-bit longs are "typical". No, we aren't quite there yet, as of today. But I'd wager in 10 years, 32 bit longs will seem quaint.
  • Harrow 2008-11-28 14:44
    Daniel:
    You can complain about that to me when you do an accounting program in 4 KB RAM.
    ...On a CPU that does not even have a subtract instruction.

    I would not want to go back to those days, but really -- I cannot accept code quality judgements from people who need a Java EE IDE, 2 GHz, and 512 MB just to produce "Hello, world".

    -Harrow.
  • timb 2008-11-28 15:00
    m:
    MAG:
    The real WTF is people thinking that having no comments, pointless and not so helpful variable names and a stupid function name is not a WTF.

    Also, isn't there any number format C function??


    It's a very self-descriptive function name, perfectly understandable variable names, totally obvious control flow, and only idiots who having learn PHP a week ago think they know about programming think comments in a function like that are anything but an obstruction that makes it harder to understand. Not to mention that what it does is on the level of the second hour of programming 101 class. Too bad Jake apparently hasn't heard about modulo division or positional number systems.

    That's a perfectly fine function, and guess what, sometimes there aren't standard functions available. My bet (especially given the apparent threading/overflow thing pointed out before) is that it's from embedded code, where it's totally okay to write unsafe code to save a few bytes.


    A) No it's not from embedded code

    B) If you think that function is 'fine' then I seriously hope I never have to maintain any of your code. Only poor programmers think along the lines of "if someone can't instantly understand this then they must be stupid". Adding a one line comment above a function to say what it does takes 5 seconds. Each additional programmer who comes to see this function has to decide what it does and that will ALWAYS take longer than 5 seconds. When you are scanning your way through MILLIONS of lines of code, stopping for even 30 seconds to figure out what EACH function does slows down the whole operation.

    People who fail to document their code should be banned from life.
  • Numeromancer 2008-11-28 15:05
    JPhi:


    And for all of you asking "isn't there a library function to do that", the answer is NO. The C language is about getting stuff done...


    ...the same stuff, over and over, a hundred ways, all alike excepting the variety of bugs which cause program crashes in special cases. Brillant!
  • emcoffey3 2008-11-28 15:10
    In terms of code correctness/performance/efficiency, this function seems pretty solid. The only thing I immediately noticed was that he could have saved 3 bytes by using bool instead of int for the "neg" variable, seeing as how it only has 2 possible values. Then again, this function may very well pre-date the bool type.

    As for people not understanding the code, I can certainly see why. If you're not familiar with C++ pointers, the pre-increment operator, and "C-strings", then this probably won't make a bit of sense to you. And the vaguely named variables and lack of comments won't help, either.

    It isn't pretty, but it's not even close to the worst I've seen. I'm so glad I don't write code in C++ anymore... just looking at this gave me chills.
  • Clarence Odbody 2008-11-28 15:15
    So timB's defense is that the code is not commented? That's a WTF? And what happened to the "horribly implemented" comment? No longer defending that?
  • flaggy 2008-11-28 15:34
    timb:
    m:
    MAG:
    The real WTF is people thinking that having no comments, pointless and not so helpful variable names and a stupid function name is not a WTF.

    Also, isn't there any number format C function??


    It's a very self-descriptive function name, perfectly understandable variable names, totally obvious control flow, and only idiots who having learn PHP a week ago think they know about programming think comments in a function like that are anything but an obstruction that makes it harder to understand. Not to mention that what it does is on the level of the second hour of programming 101 class. Too bad Jake apparently hasn't heard about modulo division or positional number systems.

    That's a perfectly fine function, and guess what, sometimes there aren't standard functions available. My bet (especially given the apparent threading/overflow thing pointed out before) is that it's from embedded code, where it's totally okay to write unsafe code to save a few bytes.


    A) No it's not from embedded code

    B) If you think that function is 'fine' then I seriously hope I never have to maintain any of your code. Only poor programmers think along the lines of "if someone can't instantly understand this then they must be stupid". Adding a one line comment above a function to say what it does takes 5 seconds. Each additional programmer who comes to see this function has to decide what it does and that will ALWAYS take longer than 5 seconds. When you are scanning your way through MILLIONS of lines of code, stopping for even 30 seconds to figure out what EACH function does slows down the whole operation.

    Seriously, you probably don't even need to see its code in order to understand what it does if the rest of the file is well written. Even with that name, it's a local function that's probably used in a couple places where you'll print a number to the user. If you have seen the application running it should already be pretty obvious what the function does. If not, just browsing through the function gives you enough understanding. Droping a small comment like /* 1000 -> 1,000 */ or giving the function a better name is good, but certainly not crucial. Certainly not a WTF. I think you just have taken your teacher's opinions on programming a tad too seriously.

    People who fail to document their code should be banned from life.

    Along with people who takes "few minutes" to figure out what that function does (and not very well, I might add).
  • timb 2008-11-28 15:36
    Clarence Odbody:
    So timB's defense is that the code is not commented? That's a WTF? And what happened to the "horribly implemented" comment? No longer defending that?


    Speak to Jake Vinson about his comments. The fact that it lacks any kind of documentation isn't the only problem with this code. The function name for a start is very bad. nice_num could mean many things. The idea that you have nice numbers implies that you have bad numbers. Personally my implementation of nice_num would only print out values that are divisible by 12 as those are the kind of numbers I consider nice. It's just a long formatted to a string for the user to see and there are better ways to name a function that does that. The variable names are also awful. Single letter variables should have been banned at the same time as slavery. The smallest permissible variable name should be idx, which accurately portrays it's purpose in it's name.
  • themagni 2008-11-28 15:40
    eb0ny:
    sprintf(buffer, "%d.%d", n/100, abs(n)%100);

    Enough said.


    You're the kind of person we write about here on the Daily WTF. Never mind the failures in that code (just FYI, don't ever post code online in a coding forum unless you've tested it.); let's look at the general idea you're working on there.

    What you have to remember is that the code you write is never seen by the computer. It's compiled. That compiled machine code is what tell the computer to do something.

    When you pull out a huge function like sprintf, you're going to get SLOW performance. Even though it LOOKS like it's a one-line instruction, you're looking at hundreds of lines of code once the compiler's done with it. This is true with ANY string manipulation. Leave it the hell alone if you want speed. Debugging statements can be the worst thing you can do - the slow-down resulting from the printing can change how the code interacts, and often these "load-bearing prints" are left in, because the developer doesn't know how to fix the race condition.

    Remember that one line in your code isn't one instruction, and the more you pile on, the more likely it is that your code will be interrupted and write random values to random places. I've had huge bugs caused by a macro that took up a total of three lines.

    It was to fix a problem with analog ports on a PIC16F88. Any write would turn all the pins off, so it was fixed with a shadow register. To set a pin, it would read the existing PORT A, then set the pin, then write the shadow register to the physical port.

    Occasionally, an interrupt would be triggered between the read and write instructions, putting random data on the port, causing the system to fail.
  • Jeltz 2008-11-28 15:48
    I only see one problem with that codem the commonly forgotten MINT_MIN. You should either treat it separtly or not negate the input, the former probably being the better idea.

    The code could use some cleaning up and a short comment and fixing of this but but there is nothing that makes this a WTF.

    And single letter variables are not bad in any way as long as they are few and their scope is short. I have never understod this fobia. Just because something can be abused doesn't mean it is bad. These days there is actually more abuse of long variable names than there is of short. I have seen programs rendered unreadbale from both too much logn variable names and too many one letter variables. Everything has to be used with common sense and in moderation.
  • Jeltz 2008-11-28 15:51
    MINT_MIN, mmmmmmm

    Obviosuly I meant MIN_LONG.
  • ysth 2008-11-28 16:14
    MAG:
    The real WTF is people thinking that having no comments, pointless and not so helpful variable names and a stupid function name is not a WTF.

    Also, isn't there any number format C function??
    But the implementation is criticized, not the lack of comments and short variable names.

    Yes, there's printf, which does (for 10+ years now, anyway) support adding a locale-dependent thousands-grouping character. But that's a pain to deal with when you don't want to be locale-dependent.
  • Asiago Chow 2008-11-28 16:19
    timb:

    B) If you think that function is 'fine' then I seriously hope I never have to maintain any of your code. Only poor programmers think along the lines of "if someone can't instantly understand this then they must be stupid". Adding a one line comment above a function to say what it does takes 5 seconds. Each additional programmer who comes to see this function has to decide what it does and that will ALWAYS take longer than 5 seconds. When you are scanning your way through MILLIONS of lines of code, stopping for even 30 seconds to figure out what EACH function does slows down the whole operation.


    Why would you be going through a million lines of code bottom up? You'll just confuse yourself and blow a gasket.

    Structured code makes much more sense from the top down, seeing a high level function and then figuring out what each part does if needed. That way you have a context and know what sort of problem the previous programmer was trying to solve.

    How hard is this to figure out?


    int current_score = 0;
    ...
    printf("Your score is: %s\n", nice_num(current_score));



    You don't need a comment more code to know what is going on. Not unless there is a problem with how some numbers are printing -- which doesn't look, from the function posted, likely.
  • eyepatch 2008-11-28 16:33
    Agree'd. Not an enterprisey solution.
  • Errorsatz 2008-11-28 16:40
    This code is just fine:
    static char *nice_num(long n) ...

    This code would be a WTF:
    // String formatting library for use with all threading
    // configurations, countries, and future number sizes.
    static char *nice_num(long n) ...

    If you're writing a general purpose library to be used for years to come, then you should make sure your code handles all possible cases. If you're writing a function for use in a single program with defined parameters, then doing so would be a waste of time, space, and quite possibly obfuscate the code unnecessarily.
  • Will 2008-11-28 16:50
    Unacceptable solution!!!!
    How can you name this function "fine"?
    It's horrible- no XML configuration, no Web Services interface, can't be accessed via RMI, and, the most terrible thing - all done in one layer, you need a proper render layer(characters rendering), processing layer - math, and data layer, remote server that contains character codes, separator codes, etc.
    This should be combined together using Web Services, and this should be designed as scalable system, better to have few processing layers, and load balancing before them.
    This will be a real solution
  • hexatron 2008-11-28 16:57
    For the "isn't there a function that already does this" crowd:

    I just checked the source for Microsoft's C compiler function xtoa, which is the common working part of itoa(), ltoa(), etc.
    It is just about identical to the WTF code above, except for the comma stuff. The differences are:
    1. The number is given in two arguments--an unsigned value and a sign (absolutely needed for ultoa())
    2. The buffer and its size are arguments too.
    3. The result is computed backwards (smallest digit at the start of the string), and then the result string is reversed in place.

    Imagine that! The 'system function' isn't elfin magic at all. It's the same thing you've been despising.
  • Nazca 2008-11-28 16:58
    GettinSadda:
    Nazca:
    GettinSadda:
    One improvement that I would probably make is to make the loop infinite with an if(!n) break before adding the comma as this means that you don't have to remove it afterwards.


    Which in most numbers is less efficient, since you're doing an extra comparison every loop, instead of a single comparison and increment at the end.
    Actually if you look at the original code, the only changes are adding a conditional jump after the division by 10 (which will be very low cost as the test for zero will have probably been calculated by the division) and change the conditional jump at the end of the loop to an unconditional (this conditional is likely to be a more expensive than above as the manipulating of d means that you will now need to re-test the value of n). The upshot is that most systems should be faster doing the test in the middle.


    Ah, yes, apologies, I misread what you intended. That'll teach me to skim when tired.

    Yes, you are quite correct that that change would nicely.
  • Rambaldi 2008-11-28 17:10
    emcoffey3:
    In terms of code correctness/performance/efficiency, this function seems pretty solid. The only thing I immediately noticed was that he could have saved 3 bytes by using bool instead of int for the "neg" variable, seeing as how it only has 2 possible values. Then again, this function may very well pre-date the bool type.

    As for people not understanding the code, I can certainly see why. If you're not familiar with C++ pointers, the pre-increment operator, and "C-strings", then this probably won't make a bit of sense to you. And the vaguely named variables and lack of comments won't help, either.

    It isn't pretty, but it's not even close to the worst I've seen. I'm so glad I don't write code in C++ anymore... just looking at this gave me chills.


    Standard C doesn't have a bool type.

    There has been a lot of discussion about 32/64 bit longs, some architectures have 16 bits longs its all a matter of the register size, and if you are seeing straight C nowadays it will either be for hardware drivers or on an embedded system so you are more likely to encounter these shorter registers.
  • John Beaner 2008-11-28 17:20
    makes pretty good sense to me when you think about it.

    jess
    http://www.online-privacy.cz.tc
  • Peter Tax 2008-11-28 17:25
    Maybe I've seen too much bad code in my life, but apart from using a global variable prtbuf instead of passing this as an argument, this function doesn't look that horrible to me.
  • emcoffey3 2008-11-28 17:29
    Rambaldi:

    Standard C doesn't have a bool type.


    It was added in C99.
  • frustrati 2008-11-28 17:40
    using goofy modulus math

    That is just such a stupid comment that it is funny. Let's have it again:
    using goofy modulus math

    Pure cluelessness.
  • lol 2008-11-28 17:51
    fixed n=-n bug:

    1.
    [ph] [tehbox] [~] ./x 123
    2.
    pre : -i=123 neg=0
    3.

    4.
    post: i=0 s=1,2,3
    5.
    [ph] [tehbox] [~] ./x -123
    6.
    pre : -i=-123 neg=0
    7.

    8.
    post: i=0 s=-1,2,3
    9.
    [ph] [tehbox] [~] ./x -128
    10.
    pre : -i=-128 neg=0
    11.

    12.
    post: i=0 s=-1,2,8
    13.
    [ph] [tehbox] [~] cat x.c
    14.
    #include <stdio.h>
    15.

    16.
    void main(int c, char **v) {
    17.

    18.
    signed char i = atoi(v[1]);
    19.
    int neg = 0, d = 1;
    20.
    char s[10];
    21.
    char *b = s;
    22.

    23.
    b+=10;
    24.
    *--b = '\0';
    25.

    26.
    //i < 0 && (neg = 1, i = -i);
    27.

    28.
    printf("pre : -i=%d neg=%d\n", i, neg);
    29.

    30.
    do { *--b = '0' + (int)(i%10<0?i%10*-1:i%10);
    31.
    i /= 10;
    32.
    --d == 0&& (*(--b) = ','), d = 1;
    33.
    if (i<0 && i>-10) neg=1;
    34.
    } while (i);
    35.

    36.
    *b == ',' && b++;
    37.

    38.
    if(neg) *(--b) ='-';
    39.

    40.
    printf("\npost: i=%d s=%s\n", i, b);
    41.

    42.
    }
  • Bobblehead Troll 2008-11-28 17:55
    I hate mandatory invisible default locales. They are a real WTF.

    They are the #1 way to make programs that fail more or less inexplicably. For instance there was this one tool program that saved its configuration data by serializing it into XML using a some common library. It also had a default configuration when installed. Only in a different country it always crashed on startup... with a NumberFormatException since the locale-specific decimal separator was different and the configuration had plenty of floating-point numbers.

    It is pretty stupid when locale-infested string classes may arbitrarily decide that A and  and Á are actually the same letter and software subtly behaves differently depending on location, with no way to change it apart from creating your own string class. Not to mention the software overhead... in most C++ STL implementations, an innocent-looking '#include <string>' actually brings in around 300k of headers as well.

    I would quite prefer this ""wtf"" code.
  • lol 2008-11-28 17:57
    http://pastebin.com/m78db4adf
  • Josh in California 2008-11-28 18:02
    Did you really, actually make fun of the fact that this algorithm CORRECTLY divides by 10 rather than 1000?

    AHAHAHAHA!

    Epic fail.
  • Adam 2008-11-28 18:13
    This post is an embarrassment to the author.
  • Mike 2008-11-28 18:21
    actually the posted code is a rip from an ANSI C92 string formating function
  • JS 2008-11-28 18:42
    Sometimes I am really glad I have my ED instruction. ;-)
  • modo 2008-11-28 18:47
    JPhi:
    GettinSadda:


    Remind me again just how you get commas in numbers using standard printf?


    ...

    And for all of you asking "isn't there a library function to do that", the answer is NO. ... If I was faced with the comma problem, I would have done exactly what the OP(rogrammer) did.

    ... program that was done quickly an efficiently.

    sprintf("%'20ld", n) ..?

    Of course this C might not be ISO, and so might not have the "'" modifier. So, the answer is maybe.
  • BJ Upton 2008-11-28 19:00
    Osno:
    I got the meaning of the function on my first read, and I'm not even a C programmer. Ergo, comments would be redundant here.

    I'd change the name (a little) to pretty_print, or something more meaningful, and solve the few minor defects the function has, but that's it.

    I think we're way past the "uses foo as a name function" WTF. If we're back to posting stuff because of bad naming conventions, plus the ban on TopCoder, this site is doomed.


    I miss TopCod3r. He was one of the best things on the site.
  • BJ Upton 2008-11-28 19:01
    JS:
    Sometimes I am really glad I have my ED instruction. ;-)


    See your doctor if your instruction last longer than 4 hours.
  • Rick 2008-11-28 19:01
    To the OP:

    Apparently you've never actually tried to convert an integer to a C-string before. This is not goofy or "WTF." It is a well-known and elegant algorithm. Look up the wikipedia entry for the function "itoa" http://en.wikipedia.org/wiki/Itoa

    Go an implement the commas the way that you described. I will laugh at your result: a base-1000 representation of the number instead of a decimal representation.
  • Programmer 2008-11-28 19:27
    Funny that you should make fun of that code -- because it actually tells me more about you than the guy who wrote it:

    - you an lack adequate grasp of simple mathematical
    constructs like modulus

    - you are not very good at reading and understanding code
    (it took me about 5 seconds to figure out what the code
    did and a few seconds more to verify it didn't have any
    bugs that were very obvious. And I suck at reading
    code. You must be really, *really* slow)

    Of course, the guy who wrote it should have anticipated that even a short snippet like that might confuse the abnormally obtuse and added a comment that spells out what it does. And I agree the name should reflect better what the function does.

    But that still doesn't change the fact that you are an incompetent idiot.

  • lolwtf 2008-11-28 19:28
    The only problems here:
    1) Uses a global buffer. This is a WTF.
    2) Buffer overflow on 64-bit systems.
    3) A bit ugly. Could use some comments and nicer names.

    Gieron:
    Paolo G:
    bitpirate:
    The real WTF is assuming that all locales use "," for their thousands separator. "1.000.000" is common for a million in Europe.

    Yes, dot is the standard in continental Europe (which uses a comma instead of a decimal point), but the comma is used in the UK.

    In Sweden we use spaces: 1 000 000
    That must get fun when you have two numbers next to eachother separated by spaces.
  • Flow 2008-11-28 19:29
    Evo:
    Moo:
    The largest possible string for a given input is "-2,147,483,648\0", which is 15 characters, so there is no buffer overflow.


    I agree with you in everything but this sentence. The size of long may be anything, as long as it is at least as long as an integer. It may be several megabytes for all you'd know. In practice, it will be either 4 or 8 bytes. On my 64 bits processor, it's 8 bytes, which makes the largest possible string:
    "-2,305,843,009,213,693,952\0"
    which is 27 characters long.


    Sure. But since we already can assume it's embedded code, let's also assume the engineer knows the prcoessor is 32-bit, and only 32-bit for now. So therefore all that's missing is a comment to this effect.
  • dk 2008-11-28 19:31
    Function is perfectly fine...
    WTF is this whole thread with 64bit longs, multithreading, %100 instead of %10 etc, not the code pasted by the author.

    Cheers to everyone.
  • Programmer 2008-11-28 19:36
    So Jake Vinson is a lead developer and he can't even understand something as simple as formatting an integer value as a string. And he has been at it for 10 years.

    The real WTF is what he does as a lead developer. Who on earth would hire someone who after ten years isn't even smart enough to understand a piece of code any college freshman with a programming 101 should be able to grasp.

    Where have all these idiots on thedailywtf come from? There used to be smart people here. At least occasionally.
  • zzo38 2008-11-28 19:39
    A few minutes? No, it takes a few seconds to figure out.
  • Dennis 2008-11-28 19:52
    eyepatch:
    Agree'd. Not an enterprisey solution.


    Is that a contraction of "agree would"?
  • Jeltz 2008-11-28 19:58
    A sampel solution for how to solve the common bug with INT_MIN in this code. But a common bug in a border case is not a WTF. As you see this code is a bit simpler than the alleged WTF and would still be easier to read even when the commas are added. But it is just a minor bugfix and clean up. Nothin seriosuly wrong with the alleged WTF.

    http://clc-wiki.net/wiki/K%26R2_solutions:Chapter_3:Exercise_4
  • methinks 2008-11-28 20:12
    JPhi:
    We don't want this to be like that English nazi site with 1000's of posts making fun of signs that use "your" instead of "you're".


    1) What's wrong with pointing out stupid mistakes? It might help preventing more of the same in the future.

    2) Perhaps we in Europe's german speaking countries are a little bit more sensitive, but I always find it disturbing to use the term "Nazi" in connection with every-day stuff like spelling or the like.
    What's wrong with words like "pedant" or "stickler"?
  • methinks 2008-11-28 20:39
    Jeltz:
    And single letter variables are not bad in any way as long as they are few and their scope is short. I have never understod this fobia.


    I do agree about the variables. But single letter transcriptions of certain greek letters are definitely not ok ;o)
  • TopCod3rsBottom 2008-11-28 20:54
    Programmer:
    Where have all these idiots on thedailywtf come from? There used to be smart people here. At least occasionally.
    Hey! *I* will do the trolling at this venue, thankyouverymuch... Go find another bridge to lie-in-wait under.
  • Nobody 2008-11-28 20:57
    shepd:
    Although, once we grow up, we drop the pretentiousness and just do what the USians do.


    And in return we USians are seriously considering you for our 51st state (minus Quebec of course)
  • TopCod3rsBottom 2008-11-28 20:57
    lolwtf:
    Gieron:

    In Sweden we use spaces: 1 000 000
    That must get fun when you have two numbers next to eachother separated by spaces.
    Duh! In that case they use a comma...
  • JimBob 2008-11-28 21:03
    Daniel:

    It looks like old code.

    Um, what does that mean? And how can you tell?
  • pile 2008-11-28 21:06
    My version of "perfectly good code" is:

    * documented
    * has a name of the function that is more representative of what it is
    * says what it does in the header
    * isn't called "_num" when it outputs a pointer to a string
    * has some convention for handling foreign formats (like . instead of , as a separator)

    I agree. Crappy code. Especially if you have to waste time trying to figure out what it does. Programmers like this give C a bad name.
  • Anonymous 2008-11-28 21:45
    pile:
    My version of "perfectly good code" is:

    * documented
    * has a name of the function that is more representative of what it is
    * says what it does in the header
    * isn't called "_num" when it outputs a pointer to a string
    * has some convention for handling foreign formats (like . instead of , as a separator)

    I agree. Crappy code. Especially if you have to waste time trying to figure out what it does. Programmers like this give C a bad name.


    Again: lack of commenting and bad naming hardly makes for a WTF worth of mentioning here.

    Maybe Jake is TC. I mean, look at the coincidences: TC is a lead developer (most of the time) that posts absurd WTF solutions as perfectly acceptable solutions while Jake is a team lead that posts perfectly acceptable solutions as WTFs.

    And also, since when locale is absolutely mandatory on every piece of code? Last time I checked, locale may or may not be a requirement. Even if this is not embedded code, the coder may have a very good understanding of his target PC and may not needed this to be portable through locations and time (meaning, 64 bit may be overkill for this code). Even though adding locale is trivial to implement, it still will make the code (or the calling code) more confusing and slow. It may even take it to the point where it takes a few minutes to understand it...
  • neophrene 2008-11-28 22:35
    Rambaldi:

    Standard C doesn't have a bool type.


    WAOUH

    I just found a time machine to travel 10 years in the past. Yeepee!

    (and i also found TRWTF: it's Rambaldi)
  • anon 2008-11-28 22:44
    this code will run on anything from a $2 microcontroller to a billion dollar supercomputer.

    damn it, if its not leaking, slow or giving the wrong answer than WTF is wrong with it?

    fuck you style idiots with your hands on your hips complaining about modulus math. you want comments? here's one: step off!
  • pythonic 2008-11-28 22:45

    /**
    * formats number with comma delimiters, implemented with
    * go-faster stripes, but uses ~4k memory.
    */

    static char* format_c03d = ",000,001,002,...";

    static char*
    format_number_with_comma_delimiters(char *dst, int64_t number)
    {
    int64_t n;

    dst += 32;
    *dst = '\0';
    n = number;
    while (n) {
    dst -= 4;
    *(u_int32_t*) dst = *((u_int32_t*) format_c03d + abs(n % 1000));
    n /= 1000;
    }
    while (*dst == '0' || *dst == ',')
    dst += 1;
    if (*dst == '\0')
    *--dst = '0';
    else if (number < 0)
    *--dst = '-';
    return dst;
    }

  • joeyadams 2008-11-28 23:13
    I've been programming these kinds of confusing but efficient functions for a few years now. I still have trouble writing and rereading them. However, I do use a few conventions:

    Start, end pointers
    char *s, *e; //refers to a string starting at s and ending right before e
    /* ... */
    while (s<e) {
    char c = *s++;
    }

    Pointer, remaining
    element *item;
    unsigned long count;
    /* ... */
    for (;count--;item++) {...}

    Though these sort of things may be convoluted, they are C, and they are efficient :)
  • Rick 2008-11-29 00:41
    summerian:

    No, it does not have to process one digit at a time. You just check if the remainder is below 10 or 100 to add leading "00" or "0".

    But it guess it's the same either way.


    Apparently you don't understand how the code works. Yes you DO have to process it one digit at a time.

    *--buffer = '0' + (n % 10);

    There's no ASCII code for '11' or '100' or '12'. There's only '0' to '9'. If you go modding and dividing by 1000 then the part that converts the number to a character won't work.
  • Kragen Javier Sitaker 2008-11-29 01:32
    Pursuant to Jake Vinson's suggestion, I restructured the code to use an outer loop that divides by 1000. I understand it's a bit risky to take programming advice from someone who's so junior they don't understand why someone would use the "%" operator in a decimal print routine, but I thought I would see how it went. I do not think the result is an improvement in clarity.


    /* nested loops, as suggested by Jake Vinson */
    static char *nested_nice_num(long n)
    {
    int neg = 0, d = 3;
    char *buffer = prtbuf;
    int bufsize = 20;

    if (n < 0)
    {
    neg = 1;
    n = -n;
    }
    buffer += bufsize;
    *--buffer = '\0';

    for (;;) {
    int nd = n % 1000, ii;
    n /= 1000;
    for (ii = 0; ii < 3; ii++) {
    *--buffer = '0' + (nd % 10);
    nd /= 10;
    if (!nd && !n) break;
    }
    if (!n) break;
    *--buffer = ',';
    }

    if (neg) *--buffer = '-';
    return buffer;
    }


    I agree with the other comments that ① the original is a perfectly reasonable piece of code for environments where you don't have a sprintf available; ② it's slightly trickier than it needs to be, probably in order to be fast; ③ a sprintf version would be simpler, if not faster; ④ the original poster is an idiot; ⑤ a one-line comment
    /* Convert signed integer to comma-separated decimal string */
    would be a major improvement, as would renaming the function "comma_sep".

    Here's a version using sprintf (and still using statically-allocated 20-byte buffers, to compare apples to apples). As a special bonus, it formats -2,147,483,648 as "-2,147,483,648" instead of "-.,/,),,(-,*,(" (on an ASCII machine).


    #define BUFSIZE 20
    static char prtbuf[BUFSIZE];
    static char prtbuf2[BUFSIZE];

    static char *sprintf_nice_num(long n)
    {
    char *s = prtbuf2, *t = prtbuf;
    int d = sprintf(prtbuf, "%ld", n);
    if (n < 0) d--, *s++ = *t++; /* no comma after the minus! */
    for (; d--, *s++ = *t++;) if (d && !(d % 3)) *s++ = ',';
    return prtbuf2;
    }
  • Shinobu 2008-11-29 01:35
    Nazca:
    Heck... I suppose you'll find this a wtf:

    const int ALLOW[256] =
    
    {
    /* 0, 1, 2, 3, 4, 5, 6, 7, 8 9, A, B, C, D, E, F */
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*0x*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*1x*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*2x*/
    -747,-747,-747,-747,-747,-747,-747,-747,-747,-747,0x00,0x00,0x00,0x00,0x00,0x00, /*3x*/ /* 0 = 0x30, 9 = 0x39 */
    0x00,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747, /*4x*/ /* A = 0x41 */
    -747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,0x00,0x00,0x00,0x00,0x00, /*5x*/ /* Z = 0x5a */
    0x00,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747, /*6x*/ /* a = 0x61 */
    -747,-747,-747,-747,-747,-747,-747,-747,-747,-747,-747,0x00,0x00,0x00,0x00,0x00, /*7x*/ /* z = 0x7a */
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*8x*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*9x*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*Ax*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*Bx*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*Cx*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*Dx*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, /*Ex*/
    0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 /*Fx*/
    };
    char chk_validity(char *s)
    {
    static char *c;
    for (c=s;*c;c++) if (ALLOW[c] != 0) return (char)0;
    return (char)1;
    }


    Addendum (2008-11-28 13:52):
    for (c=s;*c;c++) if (ALLOW[c] == 0) return (char)0;

    *coughs, looks sheepish, and hopes no one replied yet*
    You forgot to dereference that pointer, spunky.

    Anyway, regarding the article itself, I'll have to join the chorus. This is definitely not a WTF.
    ‘takes a few minutes to figure out’: Well, I guess that makes me a genius, as I read through the thing in about three seconds. Or the submitter a moron. In all honesty I think the latter is more likely.
    ‘is trying to accomplish’: Accomplishes.
    ‘works backwards’: That's inherent in the problem. You simply can't do this working from the other end.
    Yes, the code has a few minor issues, as previously pointed out by others, but all in all, it's okay. Definitely not a WTF.
    Oh, and as for the ‘comments defense’: get over yourself. In this case reading the comments would actually have taken more time than reading the code. The art of commenting is also in a large part the art of knowing when to leave them out.
  • John 2008-11-29 03:19
    Robajob:
    JPhi:
    We don't want this to be like that English nazi site with 1000's of posts making fun of signs that use "your" instead of "you're".


    I do.


    I do too!

    But what REALLY puzzles me is that some writers confuse 'then' and 'than'

    It may be understandable for someone for whom English is a second or third language, but this is the kind of basic detail grasped by young kids.

    At least, I thought so.
  • owillebo 2008-11-29 03:34
    This is not C++ but that assembly like language called C
  • Mod Vinson 2008-11-29 04:04
    pythonic:

    *(u_int32_t*) dst = *((u_int32_t*) format_c03d + abs(n % 1000));
    n /= 1000;


    Hey! Stop using that goofy modulus math!
  • Watcher 2008-11-29 04:19
    So what's wrong with variable names?
    neg stands for negative, n for number, d for digit.

    Or may be you prefer code like that:

    for(int iteration = 0; iterator < max; iterator++) { ... }
  • Frzr 2008-11-29 04:50
    Nice function, try calling it with this argument:

    long x = (long)0x80000000;
    char *p = nice_num(x);
    printf("result: %s\n", p);

  • Steve 2008-11-29 05:03
    I keyed this in for a stroll down memory lane. Been a while since I used C/C++. The printf statement needs an s instead of an i to format the string, as opposed to what the person above said. Here's a working version for the mildy curious. Now I'll tinker with it to remind myself how C works.

    #include<stdio.h>

    static char prtbuf[20];

    static char *nice_num(long n)
    {
    int neg = 0, d = 3;
    char *buffer = prtbuf;
    int bufsize = 20;

    if (n < 0)
    {
    neg = 1;
    n = -n;
    }
    buffer += bufsize;
    *--buffer = '\0';

    do
    {
    *--buffer = '0' + (n % 10);
    n /= 10;
    if (--d == 0)
    {
    d = 3;
    *--buffer = ',';
    }
    }
    while (n);

    if (*buffer == ',') ++buffer;
    if (neg) *--buffer = '-';
    return buffer;
    }

    int main(){
    printf("the number is %s\n", nice_num(100000));
    return 0;
    }

    Here's the output:
    /tmp/>a.exe
    the number is 100,000



  • NiceWTF 2008-11-29 05:07
    thosrtanner:
    Why would it need to fill the buffer with spaces? It returns a pointer to the start of the string it has generated, i.e. the '-' sign or the first digit.


    You're right, I missed that. So in that case there really is nothing significantly WTF-worthy about this function.
  • GettinSadda 2008-11-29 05:34
    modo:
    JPhi:
    GettinSadda:


    Remind me again just how you get commas in numbers using standard printf?


    ...

    And for all of you asking "isn't there a library function to do that", the answer is NO. ... If I was faced with the comma problem, I would have done exactly what the OP(rogrammer) did.

    ... program that was done quickly an efficiently.

    sprintf("%'20ld", n) ..?

    Of course this C might not be ISO, and so might not have the "'" modifier. So, the answer is maybe.
    Funny, my copy of ISO/IEC 9899:1999 doesn't contain any reference to the "'" modifier - maybe your definition of ISO standard C is different to mine!
  • Pony Princess 2008-11-29 05:38
    It's pretty straighforward, it took me the time to read it to understand ; not a few minutes.
  • Terra 2008-11-29 06:16
    Other than what looks like static buffer usage (bad, but might be called for in some embedded scenarios?) and unnecessarily short variable names, I'll join the chorus of "not a WTF" here.

    It looks reasonably fast, could (admittedly) be more readable, and is a tiny tool for a tiny job. If you need some general number formatting, it is not the right tool. It should work for what it strikes out to be, and that is the goal for a tiny tool.
  • Pax 2008-11-29 06:18
    ZPedro:
    I'm going to have to join the chorus here: I don't see much of a WTF; especially the "few minutes to figure out" argument doesn't hold, for me "I came, I saw, I understood"


    I agree; as soon as I saw the "d = 3" and checked back to see that it returned a char*, I instantly thought it was a number prettifier. The ',' character down below just confirmed it for me. Total time to figure out what it did: about 5 seconds.
  • Jimmy Jones 2008-11-29 06:24
    I read straight through it, understood it, didn't see any problem.

    Where's the WTF?
  • rcc 2008-11-29 06:31
    Or, maybe, out of your league?
  • Cookie 2008-11-29 06:53
    I might have a quarrel with the rather horrible LOC-driving formatting, the global storage it uses for a buffer, the undocumented constant 20 (which is actually a bit short, think log10(2^63)+2 for sign and terminator) and the failure to put the comma under a define or (my preference) an enum. Also a minor nit that there are at least two other characters (dot and apostrophe) used for the same function, elsewhere. But not with the method.

    The ``backwards'' approach to make happen what this does is pretty much the natural one. To see why, try and come up with an elegant ``forwards'' method. In fact, I have a function that does variable base number-to-ascii (but without comma support) in the same way and it doesn't even add a terminating NUL. Why? The subsequent write function takes a length argument, and why not? The information is available already. Might as well use it.
  • Cookie 2008-11-29 06:56
    And right after I post it, I realise I forgot another: the 20 isn't just short, as commata take space too. whoops. Buffer overflow!
  • Jeltz 2008-11-29 07:20
    methinks:
    Jeltz:
    And single letter variables are not bad in any way as long as they are few and their scope is short. I have never understod this fobia.


    I do agree about the variables. But single letter transcriptions of certain greek letters are definitely not ok ;o)


    They sure are in Swedish. Thanks for pointing out my typo.
  • GettinSadda 2008-11-29 08:06
    Cookie:
    I might have a quarrel with the rather horrible LOC-driving formatting, the global storage it uses for a buffer, the undocumented constant 20 (which is actually a bit short, think log10(2^63)+2 for sign and terminator)
    ...
    And right after I post it, I realise I forgot another: the 20 isn't just short, as commata take space too. whoops. Buffer overflow!


    Oh, c'mon guys!

    In the C99 spec you find:

    ISO/IEC 9899:1999:
    — minimum value for an object of type long int
    LONG_MIN -2147483647 // -(2^31 - 1)

    strlen("-2,147,483,647") < 20

    Yes, this can be overridden by the implementation, but in many environments (especially embedded systems) the compiler implementation is fixed and well understood by those writing the code.
  • James 2008-11-29 09:29
    The WTF is, they could have just done this:

    /* must call setlocale(LC_NUMERIC, "") before calling. */
    static char *nice_num(long n) {
    sprintf(prtbuf, "%'ld", n);
    return prtbuf;
    }

    (Meaning, the whole function is unnecessary; it's better to avoid using a static like prtbuf for this anyway)

  • Gumpy Gus 2008-11-29 09:37
    I don't see any major problems with the code. Modulus math is the only way to do this, short of subtracting powers of ten. You can't do it three digits at a time without ending up with "1, 74" and the like. It's a bit flaky returning a pointer into a static, but not terribly bad.

    I usually do this with a sprintf %d, then add the commas.

  • fluffy777 2008-11-29 11:04
    You can't do it three digits at a time without ending up with "1, 74" and the like.


    ever tried printf("%03d", 42); // prints "042"

    printf can do about 80--99% of what you want and could reasonably expect it to do. If you know the magic codes.

    My favorite trick is

    printf("%*s<stuff>", indentation, "", ...args for stuff...);


    it beats

    for(int i = 0; i < indentation; i++)
    
    putchar(' ');
    printf("<stuff>", ... args for stuff ...)


    and the code sample here is not a WTF. Is the source of WTFs drying up? Has TDWTF made an impact?
  • Chris 2008-11-29 12:36
    How the hell is a function called "nice_num" not a WTF?!
  • Goplat 2008-11-29 12:40
    James:
    The WTF is, they could have just done this:

    /* must call setlocale(LC_NUMERIC, "") before calling. */
    static char *nice_num(long n) {
    sprintf(prtbuf, "%'ld", n);
    return prtbuf;
    }
    That function just writes 'ld for me. Believe it or not, not everyone uses Linux.
  • Cookie 2008-11-29 15:00
    GettinSadda:

    In the C99 spec you find:

    ISO/IEC 9899:1999:
    — minimum value for an object of type long int
    LONG_MIN -2147483647 // -(2^31 - 1)

    strlen("-2,147,483,647") < 20

    So sorry, but it's not relevant. The argument to the fn was given as "int", which still means it is entirely permissible (and it does happen) that the argument ends up being 64 bits. In that case you have a potential buffer overflow, which could've been prevented IF ONLY someone had written a larger 20. But that isn't my real complaint.

    GettinSadda:

    Yes, this can be overridden by the implementation, but in many environments (especially embedded systems) the compiler implementation is fixed and well understood by those writing the code.


    You can't on the one hand claim standards conformance and on the other hand say that everybody does something else anyway. Especially claiming common sense on TDWTF is a bit capricious.

    The problem is that "20" is not an adequate way to document the assumptions behind that particular use. Code does get ported, so documenting platform assumptions isn't a bad thing. Not doing that arguably is.
  • AdT 2008-11-29 15:01
    mscha:
    A function that has variables?? Shocking! :-P


    I was thinking the same thing. Fortunately, I was able to rewrite the beast in a language that has no variables in the first place – Haskell!


    shows_nice_num n =
    ( if n < 0 then showString "-" else id ) . foldr (.) id (
    ( intersperse (showString ",") . ((shows $ last l):) .
    reverse . map (\n -> foldr (.) id (replicate
    (3 - length (show n)) (showString "0")) .
    shows n)) (init l))
    where
    l | n == 0 = [0]
    | otherwise =
    unfoldr (\x -> if x /= 0 then
    Just (x `mod` 1000, x `div` 1000) else Nothing)
    (abs n)


    Now testing the new function is as simple as


    main =
    (getLine >>= return . shows_nice_num . read) <*>
    return "\nThis is insane!" >>= putStrLn
  • Bisom 2008-11-29 15:44
    Like someone else said, the only complaint I can think of is the divide ops ..maybe the WTF is the WTF. A meta-WTF!
  • anon 2008-11-29 17:17
    I found this snippet on the Web:

    http://www.eskimo.com/~scs/c-faq.com/stdio/commaprint.html

    Hmm, what a surprise. It uses the exact same technique, only it happens to have a bigger buffer and support for system locale.

    Multiple calls will overwrite the buffer, but that's what strdup is for.
  • ML 2008-11-29 18:20
    lolwtf:

    Gieron:

    In Sweden we use spaces: 1 000 000
    That must get fun when you have two numbers next to each other separated by spaces.

    I have a lot of CPUs! In fact, I have 96 386 CPUs!

  • JimM 2008-11-29 19:47
    Programmer:
    ... understand a piece of code any college freshman with a programming 101 should be able to grasp...
    While I'm happy to rely on people with a better grasp of C than me that this is not a WTF, it really irks me that so many people think not understanding this code indicates a lack of intelligence. Some of us took "programming 101" in the last five year, at colleges / universities that concentrated on modern OO languages (you know, the ones a lot of modern commercial software is written in), and have only a passing familiarity with C - I'm not a bad programmer, but having no knowledge of the language it'd take me a while (and possibly a textbook!) to figure out what this actually did (pointers? C-strings? Youwhatnow?!?).

    Knowledge is not the same thing as intelligence, people...
  • methinks 2008-11-29 19:52
    John:
    Robajob:
    JPhi:
    We don't want this to be like that English nazi site with 1000's of posts making fun of signs that use "your" instead of "you're".


    I do.


    I do too!

    But what REALLY puzzles me is that some writers confuse 'then' and 'than'

    It may be understandable for someone for whom English is a second or third language, but this is the kind of basic detail grasped by young kids.

    At least, I thought so.


    for puzzlig mix-ups, you might add:

    "their" / "there" / "they're"
    "its" / "it's"

    It's baffling me every time - judging from various online forums, chats etc. native speakers are more likely to make these mistakes.

    It might be that you have to learn spelling from the ground up when learning a new language (after all you continuously have to use a dictionary even for simple words in the beginning).

    Perhaps if you learn a language mainly by listening (which is the case for one's mother tongue) you tend to mix up homophones (i.e. words that sound alike) if you never get formal training and/or do not read a lot, as reading a word repeatedly helps in "anchoring" the pattern of how that word is spelled correctly.
  • lukas 2008-11-29 20:10
    Nicely written function. It's actually nice to see such a nice code written in pure C.
  • methinks 2008-11-29 20:16
    Jeltz:
    methinks:
    Jeltz:
    And single letter variables are not bad in any way as long as they are few and their scope is short. I have never understod this fobia.


    I do agree about the variables. But single letter transcriptions of certain greek letters are definitely not ok ;o)


    They sure are in Swedish. Thanks for pointing out my typo.


    They are in Swedish? I see...

    In Germany, Austria and Switzerland there was a reform of the official orthographic rules some years ago.
    Apart from some quite sensible stuff, the transcription of greek words was also "simplified", which is in part really stupid: "th" signifies a different letter (gr. "theta") than "t" (gr."tau"), after all, the same goes for "y" (gr. "ypsilon") and "i" (gr. "iota").

    These "simplifications" therefore tend to destroy information and moreover make some classic words butt-ugly ;o) (the latter being a point of personal taste, of course... "symphonie" vs. "sinfonie" is an example)
  • Procedural 2008-11-29 21:30

    There's no problem with the code; that's the kind of thing you expect to see in buffer-management code. I think the WTF is in the mind of the proposer: that all code should use APIs and libraries (to the extend that the proposer never actually saw such ordinary code before). It is a programmer's responsibility to know the algorithmic path of every byte in and out of memory; just relying on higher-level functions and leaving the gory details to the black box doesn't sound like the work of a senior to me.
  • Ken 2008-11-30 01:23
    Depending on the situation, this is may not be surprising. I have encountered situations where programmers have been forced to write such code. It could be during crunch time where they don't have the time to lookup the APIs, or the original programmer has left, leaving us with no option but to "temporarily" employ someone who we know is not qualified (in one instance, my manager), or we realize that the library that provides such functionality is not available/is too slow/consumes too much memory on the arcane target platform.

    Heck! I remember code that was originally written for Windows 3.1 where we had to code some seemingly basic functions that were available in Unix (and eventually in Windows XP onwards). We have not had the time to rewrite the 15+ year old code until now (for Vista). It's more like we really did not bother to do so as the original coder understood portability issues and coded it in such a way that it could be recompiled verbatim on any OS.

    I am sure that all of us will have a nice laugh at the old code, if I put it up, but we probably saved half a million dollars over all these years by not having to maintain it. I always encourage people to rewrite old code, if they are capable of coding and testing it properly.
  • Bryan 2008-11-30 01:32
    Zomg, you're like the python of currency
  • RTH 2008-11-30 01:45
    You use sprintf to get the number into a string, and then you simply add a comma every 3 spaces in some kind of loop... It takes all the "math" out of the equation and turns the problem into a simple string manipulation.

    Talk about making something that's, 'okay', into a wtf! What do you think sprintf is doing to get the digits in the first place? And you think futzing around shifting digits in a string is simpler than executing a few 'OOH AHH' ***MATH*** instructions?? Surely there's a limit on how ignorant one is allowed to be and still go near a computer in a programming capacity?
  • Rambaldi 2008-11-30 05:01
    neophrene:
    Rambaldi:

    Standard C doesn't have a bool type.


    WAOUH

    I just found a time machine to travel 10 years in the past. Yeepee!

    (and i also found TRWTF: it's Rambaldi)


    Best response to my incorrect statement, not sure what the equivalent is for this site but want a cookie?

    Its my bad for being a student and using non what ever the latest version of C is C compilers for embedded systems.
    *hates having to keep things on the same line when it would be much easier to understand if they were on multiple*

    If this was for an embedded system sprintf might not be an option, its less so an issue now but if you are using old hardware then the standard libraries can take up a fair chunk of your rather limited resources.
  • Bisom 2008-11-30 05:52
    methinks:

    It's baffling me every time - judging from various online forums, chats etc. native speakers are more likely to make these mistakes.


    You just wait 15 years. Then it's gonna be something like

    English style guide of 2023:

    Although "they're" is the correct writtrn form for the contraction of "they are", "their", "there" and "thei're" are in such wide usage that they are considered generally accepted standard English. This is after the NY Times finally conceded there defeat on this linguistic issue in 2019. LOL.


    And nobody will understand C code either.
  • Bisom 2008-11-30 05:54
    A flame war about number formatting and whose code has hairier balls. It's nice to see such passion still exists. :)
  • K 2008-11-30 06:24
    1. have written code that does this, so immediately obvious
    2. the group comma thing is +10 seconds mental parse time
    3. expect every C programmer to grok this in about 30 sec max
    4. bufsize = 5*sizeof(n) /* ought to be safe here */
    5. "n = -n" may trap with signed long (LONG_MIN).
    Anyway, for your IA32 homecomputor, nice_num(-2147483648) will produce junk like "-.,/,),,(-,*,(". Or, if you compile with gcc -O0 -ftrapv, it SIGABRTs. Need unsigned arithmetic here!
    This 5. could be TRWTF, except:
    6. This article itself reads like a meta-WTF. Boo!!!
  • Naveen 2008-11-30 09:24
    Seems perfectly OK to me except that it needs some comments. I have seen much more cryptic codes than this.
  • pwned 2008-11-30 10:21
    Surely this submission has to have come from TopCod3r.... havn't seen an article starting a thread like this in a while - and no real mentions of TRWTF
  • Tomtefar 2008-11-30 10:45
    ML:
    lolwtf:

    Gieron:

    In Sweden we use spaces: 1 000 000
    That must get fun when you have two numbers next to each other separated by spaces.

    I have a lot of CPUs! In fact, I have 96 386 CPUs!

    That's not a problem. What you do is insert the unit of count "st" (short for "stycken" meaning "pieces").

    The sentence ML wrote becomes:
    "Jag har många CPUer! Faktiskt har jag 96 st 386-CPUer!"
  • Bisom 2008-11-30 11:21
    Back when I was young we'd convert longs to prettified char strings by toggling binary switches.

    That's noting. All we had was a soldering iron and a bunch of magnetic relays.

    Relays, eh?? Luxury. We had a ball of string, a twig and some pine cones.

    Bah! You were lucky to have a twig! All we had was a pebble and a handful of river water.

    Etc.

    :)
  • cactus 2008-11-30 14:56
    I think the worst case for a long would require a 15 character buffer.
  • Alexis de Torquemada 2008-11-30 14:56
    K:
    5. "n = -n" may trap with signed long (LONG_MIN).
    Anyway, for your IA32 homecomputor, nice_num(-2147483648) will produce junk like "-.,/,),,(-,*,(". Or, if you compile with gcc -O0 -ftrapv, it SIGABRTs. Need unsigned arithmetic here!


    Nice catch! Reminds me of the checked integer arithmetic functions I wrote not too long ago (in C++). The division function can throw std::overflow_error on two's complement architectures. :-)
  • Jimbo 2008-11-30 16:58
    So it's not a WTF - get over it...Ignore it...wait for another one - tomorrow. (Or better still, F-off somewhere else if TDWTF annoys you so much that uyou feel you have to complain that it's not what is used to be)

    FFS the one thing that Sh!ts me most on this site is the idiots that feel the need to post (often it's quite obvious they haven;t read ANY of the other posts) with "That's not a real WTF..." etc, etc, etc....

    And, of course, the obligatory "Why a child of four could understand that code". Granted. But once the 15th person starts posting that, it seems more of a "I'm smart, I get it too" rather than a genuine observation...

    I can't help (after reading most comments about that particular days article on TDWTF almost daily) that the IT industry is full of F-wits who all think they have to prove how smart they actually are (which to me implies that they aren't that smart at all and realise it - they are trying to prove they are smart to themselves as well as to the world {or what little of the world reads TDWTF comments, at least}).

    If you don't like the article IGNORE it...we all make mistakes (some more often than others), and perhaps Alex (oops, Jake) was half asleep when he skimmed over this one and put it up...Deal with it....


    [/rant]

    Quite appropriately, captcha = genitus which (no not really, before you look it up) means 'Smart Dick'
  • Goofy Modulus Math 2008-11-30 18:01
    This code provoked a "WTF" reaction from me, but only because I'm not a C programmer. Anyone (like me) whose primary environment is weakly-typed languages where converting numbers to strings is a simple matter of sticking '""+' in front, might take a while to understand what was going on. I'm quite pleased that I managed to figure it out - but then I use goofy modulus math on a regular basis (for example to set different properties on alternating rows in a table ;) )

    For me it was a "WTF" - but not the kind of WTF that this site is about.
  • chrome 2008-11-30 19:03
    You guys are hilarious. Someone makes a post saying 'it takes some time to understand this code' so of course you all set out to prove how smart you all are by showing how quickly you understood it.

    Why anyone would implement something like this (or congratulate the original coder on his cleverness and obviousness) and not use strfmon() (found with <30 seconds of googling) boggles the mind.

    My only conclusion is that the great majority of you have your head so far up your own arses that you wouldn't see a bad idea if it came up and segfaulted on you.
  • Sager 2008-11-30 20:34
    chrome:

    Why anyone would implement something like this (or congratulate the original coder on his cleverness and obviousness) and not use strfmon() (found with <30 seconds of googling) boggles the mind.


    Another whos's clearly read the comments before posting....

    My little microcontroller has great need for all those monetary functions...lets use that library....Oh bugger - now I haven't room for anything USEFUL...Oh well, it solved he problem at hand....
  • Sager 2008-11-30 20:36
    In fact, better still, given that someone will point out there's probably little use for formatted numbers on a microcontroller, lets try adding that library onto my mobile phone....
  • xianthax 2008-11-30 21:49
    whats wrong with you guys? there has to be a WTF here!

    no calls to poorly implemented standard libraries? FAIL

    not linking in 50kB of code to perform a simple task? FAIL

    writing novel, simple AND fast code? FAIL

    in all seriousness, anyone who thinks his _implementation_ is a WTF, you are the cause of ugly, clunky software, please raise your hand so i may make note and ensure i never hire or work with you.

    x
  • Goplat 2008-11-30 22:07
    chrome:
    Why anyone would implement something like this (or congratulate the original coder on his cleverness and obviousness) and not use strfmon() (found with <30 seconds of googling) boggles the mind.

    My only conclusion is that the great majority of you have your head so far up your own arses that you wouldn't see a bad idea if it came up and segfaulted on you.
    strfmon is not part of the C or C++ standards, and it doesn't exist in the implementation that I use. I guess I "have my head up my own arse" because I prefer code that actually compiles?
  • jspenguin 2008-11-30 22:31
    Here's my version:


    /* The separator character for your locale. You may change this to a
    reference to a global variable if you want dynamic locale. */
    #define SEP_CHAR ','
    #define FORMAT_BUFSIZE ((sizeof(long) * 4) + 4)
    typedef char format_buffer[FORMAT_BUFSIZE];

    static char *_format_num(unsigned long n, char* buffer, int is_signed)
    {
    /* C89 doesn't have a boolean type, so we treat
    the value '0' as false and '1' as true. */
    int neg = 0;

    /* A digit counter. We count from 3 down to 0
    instead of the other way because it's
    easier for the processor to compare with
    0 than with 3 -- in fact, most instructions
    do this automatically by setting the zero
    flag if the result is 0. */
    int d = 3;

    /* If we're treating it as signed, check if it's less than zero. */
    if (is_signed && ((long)n < 0))
    {
    /* See the comment for 'neg' above */
    neg = 1;

    /* 'n = -n' would work, but some compilers may get uppity
    about mixing signed and unsigned types. */
    n = (unsigned long)(-(long)n);
    }
    /* Move the pointer to the end of the buffer. Adding
    FORMAT_BUFSIZE puts us just past the end, so subtract two (one
    for the terminating null). */
    buffer += FORMAT_BUFSIZE - 2;

    /* Make sure the string ends with a null. */
    *buffer = '\0';

    /* Using a do { ... } while loop allows us to deal with the edge
    case of n == 0 because it forces the loop to execute at least
    once. */
    do
    {
    /* Check if we have processed three digits yet. */
    if (d == 0)
    {
    /* Reset the counter. */
    d = 3;

    /* Set the current location to SEP_CHAR, then decrement
    the pointer. */
    *--buffer = SEP_CHAR;
    }
    /* n % 10 gives us the value of the least significant
    digit. Since in C, characters are just numbers, and in
    ASCII the numbers are sequential, we can just add the
    character '0' to the digit to get its text value. We then
    assign it to the current location and decrement the pointer
    as above. */
    *--buffer = '0' + (n % 10);

    /* Now, we divide by ten so that the next-least significant
    digit becomes the least significant digit for the next
    loop. */
    n /= 10;

    /* Decrement the digit counter.*/
    d--;
    }
    while (n != 0); /* If n is zero, there are no more digits to
    process. */

    if (neg) *--buffer = '-'; /* If the value was negative, put a '-'
    at the beginning. */

    return buffer; /* Return the pointer to the beginning of the
    number. */
    }

    #define format_signed(n, buffer) _format_num((unsigned long)(n), buffer, 1)
    #define format_unsigned(n, buffer) _format_num((n), buffer, 0)



    Improvements:
    * 64-bit clean
    * No global buffer
    * Moves magic numbers and characters to #defines
    * Handles both signed and unsigned numbers, with edge cases covered.
    * Enterprisily commented so that even a non-C programmer can understand.


    #include <stdio.h>
    #include <limits.h> /* for LONG_MIN and LONG_MAX*/

    int main(int argc, char** argv) {
    long l1 = 123456789;
    long l2 = LONG_MIN;
    long l3 = LONG_MAX;
    unsigned long l4 = ULONG_MAX;
    format_buffer b1, b2, b3, b4;

    printf("l1: %s l2: %s l3: %s l4: %s\n",
    format_signed(l1, b1),
    format_signed(l2, b2),
    format_signed(l3, b3),
    format_unsigned(l4, b4));
    return 0;

    }


    jspenguin:~/myprogs$ gcc format_num.c -o format_num; ./format_num
    l1: 123,456,789 l2: -9,223,372,036,854,775,808 l3: 9,223,372,036,854,775,807 l4: 18,446,744,073,709,551,615



  • Planar 2008-12-01 03:04

    Oh, one more thing: the function breaks for MIN_LONG because the instruction
    n = -n; overflows in this one and only case.

    You can also test for MIN_LONG, and in that case add 1, call the original function, then increment the last digit. No need to bother with carry: you know it won't be 9 because MIN_LONG is a power of 2 :-)

  • obediah 2008-12-01 03:08
    JimM:
    Programmer:
    ... understand a piece of code any college freshman with a programming 101 should be able to grasp...
    While I'm happy to rely on people with a better grasp of C than me that this is not a WTF, it really irks me that so many people think not understanding this code indicates a lack of intelligence...


    You make a good point, but I think a lot of the vehemence is triggered by the stupid/ignorant claiming that if they can't understand it, it must be bad. That is stupid. :) Of course there are some people that hang around just to remind us how much faster they understand everything than everyone else.
  • tdb 2008-12-01 03:48
    So not a wtf. Taken out of context, there are some minor wtfs that were already mentioned (buffer overflow for 64-bit long, thread-unsafety), but it's entirely possible that the surrounding code makes validates them. Locking may exist on a higher level, as well as checks for numeric range. The thousands separator format for printf comes from SUSv2, not the C standard, so it's very unlikely that Microsoft's implementation includes it. Filling the buffer from the right is standard practice too, since extracting the digits with modulus starts from the least significant digit.

    I've spent the last few days rewriting all of printf's formatting (with a few custom conversions) in C++, since I'm not happy with the way iostreams work in some aspects. My int_to_str function is very similar to the one in this article, although more feature-rich.
  • Ape Monkey 2008-12-01 04:21
    helix:
    looks standard fare for an embedded firmware engineer.


    Plus it doesn't look unfamiliar for any compiler-writer, including me, who has implemented only a basic c-compiler in c (for kicks).
  • Anonymous I18N Pseudo-Expert 2008-12-01 04:27
    given that nobody (afaik) properly supports japanese number formatting (group every four digits), let alone the seriously wacky indian formatting (grouped by twos, except for the lowest order digits, which are a group of three) this is a fine start on an i18n number formatter--just make d an argument to support japan, and add another argument separating first and loop initializations of d to support india.
  • JimM 2008-12-01 05:07
    Jimbo:
    <snip>

    I can't help (after reading most comments about that particular days article on TDWTF almost daily) that the internet is full of F-wits who all think they have to prove how smart they actually are (which to me implies that they aren't that smart at all and realise it - they are trying to prove they are smart to themselves as well as to the world {or what little of the world reads TDWTF comments, at least}).

    <snip>
    There, FTFY ;^)

    p.s. jspenguin's implementation FTW!
  • scruffy 2008-12-01 05:43
    JimM:
    Programmer:
    ... understand a piece of code any college freshman with a programming 101 should be able to grasp...
    While I'm happy to rely on people with a better grasp of C than me that this is not a WTF, it really irks me that so many people think not understanding this code indicates a lack of intelligence.


    To be blunt, if somebody can't understand this piece of code, they shouldn't be pretending to be a C coder. If they ARE pretending to be a C coder then that demonstrates a lack of self awareness, humility, and intelligence.

    JimM:

    Some of us took "programming 101" in the last five year, at colleges / universities that concentrated on modern OO languages (you know, the ones a lot of modern commercial software is written in),


    oooo! OO! OO does not remove the need for algorithmic coding. And that's all that this !WTF was. Which bleeding edge, commercial, OO language could you be proficient in without seeing for loops, mod and div? Sure you might not see the scope rules or pointer arithmetic in that language, but if you don't understand those you shouldn't be working in C.

    JimM:

    and have only a passing familiarity with C - I'm not a bad programmer, but having no knowledge of the language it'd take me a while (and possibly a textbook!) to figure out.


    And would you seriously criticise someone elses code in a language that you didn't understand?
  • Colin 2008-12-01 06:02
    I think what the author meant to paste was:


    // CODING STANDARDS: NO MAGIC NUMBERS
    // http://en.wikipedia.org/wiki/The_Magic_Numbers
    #define THREE 3
    #define ONE 1
    #define NINETEEN 19
    #define ZERO 0
    #define ZERO_AS_CHARACTER '0'
    #define TEN (1 << 3) + (1 << 1)
    #define DECIMILIZER ','
    #define CONTINUE_LOOP superVariable1

    // This method determines if the given 'super' number is a 'winning'
    // number or not. If it is then the moduloproxybus needs to shift all
    // colours by 45.
    static char *win(long superVariable1)
    {
    goto start;

    loop:
    // This loop ensures that the buffer doesn't become greater than 7.
    do
    {
    *--superVariable5 = ZERO_AS_CHARACTER + (superVariable1 % TEN);
    superVariable1 /= TEN;
    if (--superVariable3 == ZERO)
    {
    superVariable3 = THREE;
    *--superVariable5 = DECIMILIZER;
    }
    }
    while (CONTINUE_LOOP);
    goto end;

    start:
    // TODO I don't think this is thread-safe. But its late and I'm tired
    // so I'm going to leave this. I'm quitting on Thursday anyway so it's not
    // my problem any more.
    // Also, Dave if you read this: LOLBUFFALOCHIPS!!!111
    int superVariable2 = ZERO, superVariable3 = THREE;
    int superVariable6 = 98;
    char *superVariable5 = prtbuf;
    int superVariable4 = sizeof(char) * NINETEEN + ONE;

    // Simple check to ensure that the variable isn't negative.
    // If it is then we need to consider whether it is prime or not.
    if (superVariable1 < ZERO)
    {
    superVariable2 = ONE;
    superVariable1 = -superVariable1;
    }
    superVariable5 += superVariable4;
    *--superVariable5 = '\0'; // The termination here ensures that the string isn't going to contain hacks from the internet.

    goto loop;

    switch (superVariable6) {
    case 98:
    prtbuf = "That's numberwang\n";
    break;
    default:
    // Don't do nothing.
    break;
    }

    goto loop;

    end:
    // Protect against buffer overflows by friggin' the pointer
    if (*superVariable5 == DECIMILIZER) ++superVariable5;
    if (superVariable2) *--superVariable5 = '-';
    return superVariable5;
    }
  • Dark 2008-12-01 06:05
    jspenguin: Your tour the force is impressive, but by calling it _format_num (with leading underscore) you impinge on the reserved namespace. No banana!
  • JimM 2008-12-01 07:02
    scruffy:
    To be blunt, if somebody can't understand this piece of code, they shouldn't be pretending to be a C coder. If they ARE pretending to be a C coder then that demonstrates a lack of self awareness, humility, and intelligence.
    Couldn't agree more. I'm going to assume this is a general rant rather than aimed at me, since I never claimed to be a C coder.
    scruffy:
    JimM:
    ... at colleges / universities that concentrated on modern OO languages (you know, the ones a lot of modern commercial software is written in) ...
    OO does not remove the need for algorithmic coding. And that's all that this !WTF was. Which bleeding edge, commercial, OO language could you be proficient in without seeing for loops, mod and div?
    None, and I didn't say that either. I said that because I'd learned modern OOP I didn't immediately understand this code. If I was doing this in one of my preferred languages, the first thing I'd do is cast the number to a String, then manipulate that. Without knowledge of the language's structure and limitations, my first response to seeing this code was "What on earth is this meant to do?".
    scruffy:
    JimM:
    I'm not a bad programmer, but having no knowledge of the language it'd take me a while (and possibly a textbook!) to figure out.
    And would you seriously criticise someone elses code in a language that you didn't understand?
    No, I wouldn't. I believe I said that: I'm happy to rely on people with a better grasp of C than me that this is not a WTF see? Nowhere in my comment did I criticise the code in this article. I just dislike people who try to claim that because I don't understand this code instantly I'm dumb. That was my only point, and I think I made it perfectly clearly.

    The fact that you need to make such an aggressive response to it suggests that maybe I touched a nerve? Or are you just another whingy internet snot trying to prove that you're better than everyone else? Or do you actually think I'm dumb because I didn't understand the code instantly (in which case I will have to dislike you, obviously...)?
  • Comma Free Locale 2008-12-01 07:13
    Goplat:
    James:
    sprintf(prtbuf, "%'ld", n);
    That function just writes 'ld for me. Believe it or not, not everyone uses Linux.
    Writing a nice_num() function yourself may make sense if the installed locales all have no thousands separator, and you don't have the authority to install additional locales. In that case, you can try futzing around with locales and format specifiers all you want, the ::sprintf() function will still steadfastly refuse to print any thousands separators. (And no, I don't use Linux either.)

    $ uname -s -r
    
    SunOS 5.10
    $ locale -a
    C
    POSIX
    iso_8859_1
    $ cat nice_num.c++
    #include <iostream>
    #include <stdio.h> // for the ::sprintf() function
    #include <locale.h> // for the ::setlocale() function

    int main(int argc, char** argv)
    {
    int num = 1048576;
    for(size_t i = 0; i < 3; ++i)
    {
    static const char* locale = NULL;
    switch(i)
    {
    case 0: locale = "C"; break;
    case 1: locale = "iso_8859_1"; break;
    case 2: locale = ""; break;
    }
    std::cout << "For locale \"" << locale << '"' << std::endl;
    if(!::setlocale(LC_NUMERIC, locale))
    std::cerr << "warning: could not set LC_NUMERIC to \"" << locale << '"' << std::endl;
    const size_t bufsiz = 1024;
    static char buf[bufsiz];
    ::sprintf(buf, "%'d", num);
    std::cout << "\twith %'d:\t" << buf << std::endl;
    ::sprintf(buf, "%d", num);
    std::cout << "\twith %d:\t" << buf << std::endl;
    }
    return(0);
    }
    $ c++ -o nice_num nice_num.c++
    $ ./nice_num
    For locale "C"
    with %'d: 1048576
    with %d: 1048576
    For locale "iso_8859_1"
    warning: could not set LC_NUMERIC to "iso_8859_1"
    with %'d: 1048576
    with %d: 1048576
    For locale ""
    with %'d: 1048576
    with %d: 1048576
    $
  • Anonymous 2008-12-01 07:22
    Cool thing about jspenguin's implementation is that now it takes longer to read for a C programmer but a non-C programmer may read the code... which is useful because?
  • Addison 2008-12-01 07:26

    JimM:

    Some of us took "programming 101" in the last five year, at colleges / universities that concentrated on modern OO languages (you know, the ones a lot of modern commercial software is written in),


    Speak for yourself. I graduated just a few short months ago and I had no trouble reading this code. There is literally nothing in that code that I didn't learn in college. Including pointers.
  • SlyEcho 2008-12-01 07:43
    Anonymous I18N Pseudo-Expert:
    given that nobody (afaik) properly supports japanese number formatting (group every four digits), let alone the seriously wacky indian formatting (grouped by twos, except for the lowest order digits, which are a group of three) this is a fine start on an i18n number formatter--just make d an argument to support japan, and add another argument separating first and loop initializations of d to support india.


    http://msdn.microsoft.com/en-us/library/system.globalization.numberformatinfo.numbergroupsizes.aspx

    BTW, Japanese uses the same format as English.
  • JimM 2008-12-01 07:53
    Addison:
    JimM:
    Some of us took "programming 101" in the last five year, at colleges / universities that concentrated on modern OO languages (you know, the ones a lot of modern commercial software is written in),
    Speak for yourself. I graduated just a few short months ago and I had no trouble reading this code. There is literally nothing in that code that I didn't learn in college. Including pointers.
    I did speak for myself. Me and anyone else who was on my course, and anyone else who attended a similar course that only taught a modern high-level language (for my sins, mine was Java!). Nowhere did I say that no-one who did CS recently would understand this code. If I'd taken different modules in my Masters I might have learned C and therefore understood this perfectly.

    Why has this comment elicited so many aggressively defensive replies? What did I say that was so offensive?!?
  • illtiz 2008-12-01 07:58
    neophrene:
    There is nothing wrong with this function. A good programmer understand it in far less than a minute. Of course it would be better to have a comment on top of it, but that's all. If your are not a good programmer (or if you are just a Java XML drone) you might of course want to fill it with storytelling comments.

    A good programmer also understand why it has been written like that.

    Indeed a good programmer already have written lots of functions like that.

    Please come back to us when you have enough programming experience, in various fields.


    Don't you mean "real programmer?" (http://www.pbm.com/~lindahl/real.programmers.html)
  • JimM 2008-12-01 08:01
    Anonymous:
    Cool thing about jspenguin's implementation is that now it takes longer to read for a C programmer but a non-C programmer may read the code... which is useful because?
    The code could be part of a legacy system, and all the C programmers could have left the organisation (in disgust, perhaps?) when they migrated the majority of their apps to .NET / Java / PickaLang. Something's gone funky with the legacy app, and now a PickaLang developer has been assigned to try to work out what's going wrong, because as far as management are concerned it's all just code, and that's what programmers write, yeah?

    I've been asked more than once to stick my head in applications written in languages I have absolutely no knowledge of, and the occasional relief of a pertinent comment is like nectar, believe me ;^) (Ok, jspenguin's may be a little verbose on the comments, but in this situation I'd always prefer too many comments rather than too few!)
  • AllUrBase 2008-12-01 08:08
    03 NICE-NUM PICTURE --,---,---,--9,ZZ.
  • illtiz 2008-12-01 08:14
    Yes, I did bother to read your monster comment.

    Nazca:


    JPhi:
    You use sprintf to get the number into a string, and then you simply add a comma every 3 spaces in some kind of loop... It takes all the "math" out of the equation and turns the problem into a simple string manipulation.


    Yeah...
    sprintf to one buffer and loop the array assigning to a second buffer which is returned.
    So you have all the overhead of sprintf and then do a manip loop anyway, with the added bonus of having to pull the data from a buffer instead of compute it.

    Talk about cutting your nose off to spite your face lol.



    Hmm. From what I gather, that wasn't implied at all. You can sprintf the current value % 1000 just the way the single digits are written into the buffer in the original snippet. The readability would improve somewhat. I fail to see other possible advantages. I agree that the high speed (which was probably intended with this code) would suffer.
  • scruffy 2008-12-01 08:17
    JimM:
    Couldn't agree more. I'm going to assume this is a general rant rather than aimed at me, since I never claimed to be a C coder.


    Exactly, more to the point any such person who then passes themself off as such a good C programmer that they can deride other people's work deserves all the lambasting they get.

    JimM:
    None, and I didn't say that either. I said that because I'd learned modern OOP I didn't immediately understand this code. If I was doing this in one of my preferred languages, the first thing I'd do is cast the number to a String, then manipulate that.

    I.E. you'd use a type conversion. Type conversion isn't a necessary or sufficient feature of an OO language, pretty much any scripting language or shell will do that!

    JimM:
    scruffy:
    And would you seriously criticise someone elses code in a language that you didn't understand?
    No, I wouldn't. I believe I said that: I'm happy to rely on people with a better grasp of C than me that this is not a WTF see? Nowhere in my comment did I criticise the code in this article. I just dislike people who try to claim that because I don't understand this code instantly I'm dumb.

    Again it's a general rant, there are a lot of people who do criticise code in languages that they don't understand, and to be honest it looked like you were defending them!

    I don't understand PERL very well, if I have to maintain some then it'll take me a couple of hours to work out what's going on, but that's work against my own lack of knowledge, not work trying to overcome an obtuse predecessor.

    JimM:
    The fact that you need to make such an aggressive response to it suggests that maybe I touched a nerve?


    Actually it was necessitated by the implication that C is archaic, and that everyone here was some sort of biggot for expecting everyone to get what for loops do.

    JimM:
    Or are you just another whingy internet snot trying to prove that you're better than everyone else?


    Let me get this straight, you're posting to a site called "The Daily WTF", a forum set up specifically for people to giggle and sneer at stupid code written by stupid people, and you have a problem with "internet snot"s ?;-)
  • mattc 2008-12-01 08:29
    Interesting to see so many people arguing over this method, I think it would be a nice feature on dailywtf to challenge people to re-write functions to see who can write the most optimal ;)
  • tdb 2008-12-01 08:32
    Dark:
    jspenguin: Your tour the force is impressive, but by calling it _format_num (with leading underscore) you impinge on the reserved namespace. No banana!

    Incorrect. Reserved names are those that start with and underscore that is followed with another underscore or an uppercase letter. As a regex: /^_[_A-Z]/. So an underscore followed by a lower case letter is safe.
  • scruffy 2008-12-01 08:45
    mattc:
    Interesting to see so many people arguing over this method, I think it would be a nice feature on dailywtf to challenge people to re-write functions to see who can write the most optimal ;)


    That could be fun, it'd be nice to see who can think up the biggest WTF code too;-)
  • Son Of Thor 2008-12-01 08:56
    This is standard C Stuff and it is almost beautiful bar the Global variable.

    I write in both C and C++ for a living (yes they are very different but in a subtle way) I usually choose straight C for small utility functions like this ( write it as a macro if possible) because if you write straight C code correctly it is the most portable code on the planet (it is basically warmed over pdp 11 asembley).

    Java people hate my java code because when I am sentenced to write java code I write it in C. As I say you can write C in any language you just have to find a way around not having pointers which is the only natural datatype there is that being on an unsigned integer value of the size that the operating system/CPU considers being a single work unit.

    I could go on about why I like to do my FOR loops backwards but you would just think I a strange or something like that.

  • JimM 2008-12-01 09:15
    scruffy:
    I.E. you'd use a type conversion. Type conversion isn't a necessary or sufficient feature of an OO language, pretty much any scripting language or shell will do that!
    Perhaps I've misunderstood what all the C programmers have said, but I'd gathered from other comments that C wouldn't let you type-cast then step through the string (or at least, that the available methods to do so are obnoxiously slow)? Hence the cunning use of the fact that if you add num % 10 to '0' you'll get the ascii point code from 0 - 9? And having to stack those directly into consecutive memory? Or have I got the wrong end of the stick?
    scruffy:
    ... there are a lot of people who do criticise code in languages that they don't understand, and to be honest it looked like you were defending them! ...

    ... Actually it was necessitated by the implication that C is archaic, and that everyone here was some sort of biggot for expecting everyone to get what for loops do. ...
    No, none of that was my intended meaning. Certainly not defending anyone else, certainly not calling C archaic (I did imply it was OLD, but I think that's fair given that it's older than me!), and I didn't mention any kind of loop in my comment (so I don't know where you've got that bit from...).

    I was simply frustrated at all the comments of the type "Anyone who can't understand this code in seconds is obviously dumb and pathetic". I'll give you that anyone criticising the code without understanding it is a fscktard, but criticising other people just because they don't understand a particular programming language is just as bad. I consider myself a decent coder so to be told that I'm dumb because I don't know C really riles me.

    Oh, and FYI I don't have a problem with internet snots in general, just whingy ones who think they're better than everyone else ;^)
  • Addison 2008-12-01 09:23
    JimM:
    I did speak for myself. Me and anyone else who was on my course, and anyone else who attended a similar course that only taught a modern high-level language (for my sins, mine was Java!). Nowhere did I say that no-one who did CS recently would understand this code. If I'd taken different modules in my Masters I might have learned C and therefore understood this perfectly.

    Why has this comment elicited so many aggressively defensive replies? What did I say that was so offensive?!?


    Mostly because it opens up a HUGE can of worms over the great divide between old school programming and new school. You said "I went to school recently (insert snide comment about the uselessness of C) so therefore I don't know how to read this code." Most people take offense to that because we know better then to assume someone CAN'T read code. Anyone who programs for a living and doesn't wind up on this site can read that code. It's just some people (like those that have learned OO languages exclusively) aren't willing to put 5 minutes of critical thinking to use. Not saying this is what you meant or even what you said, that's just how (imo) everyone took it.
  • JimM 2008-12-01 09:48
    Addison:
    Mostly because it opens up a HUGE can of worms over the great divide between old school programming and new school. You said "I went to school recently (insert snide comment about the uselessness of C) so therefore I don't know how to read this code."
    Lets compare that to my original comment, shall we:
    JimM:
    Some of us took "programming 101" in the last five year, at colleges / universities that concentrated on modern OO languages (you know, the ones a lot of modern commercial software is written in), and have only a passing familiarity with C
    No, I see no snide comments about C - just that I didn't learn it.
    Addison:
    Most people take offense to that because we know better then to assume someone CAN'T read code. Anyone who programs for a living and doesn't wind up on this site can read that code. It's just some people (like those that have learned OO languages exclusively) aren't willing to put 5 minutes of critical thinking to use. Not saying this is what you meant or even what you said, that's just how (imo) everyone took it.
    So you think assuming someone can read code gives carte blanche to then insult them if they don't completely understand that code straight away? As I said, it'd take me a while and some reference material to figure out what the code did on my own. I didn't say I wouldn't be willing to do that.

    I was simply replying to Programmer (and, by association, a number of other people generally who said things like "anyone who doesn't understand what this code does is an idiot") when s/he said "any college freshman with a programming 101 should be able to grasp <this code>". The point is, I finished my Master's just over 2 years ago. I can read this code no problem, in the sense of following the flow of it. But, having no knowledge of C, I couldn't understand the code. So it's simply not true that anyone with a Programming 101 can understand it. Read it yes, follow the flow yes, but understand it? Not unless the course taught C.

    As it is, everyone has now lost sight of the fact that I was making a specific reply to a specific part of a comment, and I predict a long, drawn-out flame war before I get sick of it, amke myself another cup of coffee, and go back to doing some real work. Life? Don't talk to me about life...
  • JimM 2008-12-01 09:56
    Son Of Thor:
    Java people hate my java code because when I am sentenced to write java code I write it in C.
    I can understand their point of view - after all, the whole concept of coding standards is to make sure everyone can read everyone's code ;^)

    On the other hand, I can also sympathise completely: when I went back to uni it took a term for the software workshop people to get me writing Java with standard namings, as I was coming from a Perl and self-taught Javascript background where all my naming conventions has _ in: so a method that in Java convention would be called getHorizontalPosition I would name get_horiz_pos. They actually started marking my assignments down after the second week because I wouldn't conform ;^(
  • scruffy 2008-12-01 10:31
    JimM:
    scruffy:
    I.E. you'd use a type conversion. Type conversion isn't a necessary or sufficient feature of an OO language, pretty much any scripting language or shell will do that!
    Perhaps I've misunderstood what all the C programmers have said, but I'd gathered from other comments that C wouldn't let you type-cast then step through the string (or at least, that the available methods to do so are obnoxiously slow)? Hence the cunning use of the fact that if you add num % 10 to '0' you'll get the ascii point code from 0 - 9? And having to stack those directly into consecutive memory? Or have I got the wrong end of the stick?


    Ah, I see where you're coming from now. C doesn't have a "String" data-type per-se. Instead we use arrays of 8bit numbers. (Each eight bit number storing a single ASCII encoded letter) C can widen and narrow data on recasts, but it won't perform algorithmic functions to do that. (C++ however does.) There are some common library functions for converting between raw binary (such as the algebraic functions use) and human-readable strings... but if you use them you have to include an entire library, which is a pain if you're working on an embedded task, or on an OS kernel.

    C describes very closely what the CPU actually does, which is why it's still popular in a lot of tasks that need direct access (like device drivers) low overheads (like OSs) and high performance (like multimedia codecs, game engines, etc).

    I do have to admit that a lot of C programmers have _massive_ superiority complexes, and that's probably due to being acutely aware of what the CPU's doing, and most of them can't imagine leaving a language to do this kind of thing for them.

    With respect if you start off learning on a language which can inject a load of functionality on a "simple" operation, then this does wrong-foot you when you come to learn C, and then the old school C guys start circling like vultures (or like while(1);)

    JimM:

    Oh, and FYI I don't have a problem with internet snots in general, just whingy ones who think they're better than everyone else ;^)


    Ah you'll be okay here then, nobody thinks that they're better than everyone else, they all know that they're better than each other;->
  • Ilya Ehrenburg 2008-12-01 10:42
    JimM:
    scruffy:
    I.E. you'd use a type conversion. Type conversion isn't a necessary or sufficient feature of an OO language, pretty much any scripting language or shell will do that!
    Perhaps I've misunderstood what all the C programmers have said, but I'd gathered from other comments that C wouldn't let you type-cast then step through the string (or at least, that the available methods to do so are obnoxiously slow)?

    I wouldn't call it a type-cast but a conversion. Of course C lets you do that (sprintf()), but why would you do something that complicated when there's a dead simple algorithm to do it in one go, and faster, too?
    That being said, the code has a few definite bad parts and a few potentially bad parts.
    The function should've been called format_number or similar, nice_num can be confused with a function concerning process priorities.
    It shouldn't use a global buffer but take the buffer as an argument.
    Hardcoding the thousands separator as ',', ignoring -MIN_LONG == MIN_LONG, possible buffer overflow if long is 64 bits and lack of thread safety are potentially bad, but may be justified here.
    Still, if
    setlocale(LC_NUMERIC,something with separators);
    sprintf("%'d",num);
    was an option, that would be preferable.
    And yes, all the "*--buf =" probably won't be obvious to someone not familiar with C, so it's okay if it takes them a couple of minutes to figure the code.
    Hence the cunning use of the fact that if you add num % 10 to '0' you'll get the ascii point code from 0 - 9? And having to stack those directly into consecutive memory?

    Cunning? I thought that was standard practice in every language where char is a numeric type (and the digits are consecutive in order, because of its simplicity I always am annoyed that the charcter after '9' isn't 'A' or 'a' in ASCII). You stack them directly into consecutive memory because it's simple to accomplish and what you finally want.
    I was simply frustrated at all the comments of the type "Anyone who can't understand this code in seconds is obviously dumb and pathetic".

    Understandable. Though I read them meaning "Any C programmer ..." which I could agree with.
  • Rambaldi 2008-12-01 11:18
    scruffy:

    With respect if you start off learning on a language which can inject a load of functionality on a "simple" operation, then this does wrong-foot you when you come to learn C, and then the old school C guys start circling like vultures (or like while(1);)


    gah, for(;;); depending on system you are implementing it for and compiler optimization it removes the test for 1 being a true value.
  • campkev 2008-12-01 14:40
    JimM:
    I just dislike people who try to claim that because I don't understand this code instantly I'm dumb.


    You don't have to understand it instantantly. But if you can't look at it for a few minutes and at least have an idea what it is trying to do, well, I won't say you are dumb, but maybe programming isn't the best career choice for you.
  • Scott 2008-12-01 14:53
    A few minor WTFs, already covered. 1. Static buffer--result is overwritten on next call, not threadsafe. 2. If a standard library is available, you can use sprintf() with your own buffer. 3. No comments; at least "// Converts int to string representation with comma separators" would make a 30 second job of reading the code into a 15 second job. Otherwise, this a fair way to do it.
  • Bega 2008-12-01 16:36
    [quote user="JimM"][quote user="Addison"]Mostly because it opens up a HUGE can of worms over the great divide between old school programming and new school. You said "I went to school recently (insert snide comment about the uselessness of C) so therefore I don't know how to read this code." [/quote]Lets compare that to my original comment, shall we:[quote user="JimM"]Some of us took "programming 101" in the last five year, at colleges / universities that concentrated on modern OO languages (you know, the ones a lot of modern commercial software is written in), and have only a passing familiarity with C[/quote]No, I see no snide comments about C - just that I didn't learn it.[/quote]
    <snip>
    [/quote]

    You don't think people might have taken offence at the bold comment above?

    It reads quite arrogant (to me, and I suspect many others) and sounds very "What would you c people know, use a real language".....
    I'm not interested in getting into an OO vs Other debate, but I noticed you were surprised your comments had caused such a ruckus...I would imagine it's comments like the above one that may have attracted all that attention (but hey, I could be wrong....)
  • m 2008-12-01 17:30
    Nice try, but:

    Jäg har 96 386-CPUar.

    (Excuse me Swedish, I'm backporting from Danish here)
  • WOW I dont wanna see your code 2008-12-01 22:03
    Goofy Modulus Math:
    Anyone (like me) whose primary environment is weakly-typed languages where converting numbers to strings is a simple matter of sticking '""+' in front, might take a while to understand what was going on.


    I'm just going to quote you, so hopefully others will see it too.
  • son of thor 2008-12-01 22:32
    JimM:
    I can understand their point of view - after all, the whole concept of coding standards is to make sure everyone can read everyone's code ;^)


    It is worse than that.

    I dont write Java that often and if I am going to do some string manipulation I am not going to know about the system.out.strings.stringbuffer object so would just roll my own.

    and I will start worrying about if something was allocated on the stack or in the heap and ague that it is the callers job to allocate the object not the called subroutine.

    And then a holy war breaks out it would properly be best to let someone with passion for java write the code.

    And dont get me started on what is in the language specification vs the class libirary.
  • Herby 2008-12-02 01:58
    Not a bad function. Yes, it has a few "flaws" but nothing serious. The even more subtle WTF is that the routine won't work (at least in a 2's complement system) at the most negative number. That is because it has no positive equivalent. A word to the wise to those who want to use the program.
  • nazlfrag 2008-12-02 06:11
    /* differences: goes forwards, directly modifies buffer at dest */

    nice_num_fwds(char* dest,long num) {

    long i;
    long j;
    long denom;
    long charcount;

    /* find number of digits in final string */

    i=num; /* i will be used as our number to convert */
    if(i<0) i=-i; /* incorrect for maximum negative */
    charcount=0; /* # of final digits */
    denom=1; /* denom will equal 10 to the power of charcount-1 */

    while(i) {
    denom*=10; i/=10; charcount++;
    }
    denom/=10; /* adjust to charcount-1 */

    /* convert the number */

    i=num;
    if(i<0) i=-i; /* again incorrect for maximum negative */

    /* and the rest explains itself :) */

    if (num<0) *dest++='-';

    while(charcount) {
    j=i/denom;
    *dest++='0'+j;
    charcount--;
    if(charcount%3==0) *dest++=',';
    i-=(j*denom);
    denom/=10;
    }
    *dest='/0';
    return;
    }

    Untested and dodgy (ie. zero fails) but just did it as a concept of how much more complex the logic gets going forwards. As it turns out, it's not as painful as I thought, but still clearly less efficient. Oh, and the article is not a WTF, but you already knew that.
  • JimM 2008-12-02 06:51
    scruffy:
    JimM:

    Oh, and FYI I don't have a problem with internet snots in general, just whingy ones who think they're better than everyone else ;^)
    Ah you'll be okay here then, nobody thinks that they're better than everyone else, they all know that they're better than each other;->
    Oh, absolutely; I am clearly far superior to you - except my knowledge of C, that is ;^)

    Thanks for the info: always nice to broaden my horizons into new languages. Not that my current work has much call for C (fricking web apps - I hate that I have a designers eye... everyone wants web apps! GAH!) but I'm sure it'll come in later (when I refuse to build any more web apps!)...
  • Woteva 2008-12-02 07:38
    Why would you want to divide by 1,000 when you're converting to a string digit by digit?
  • Tom 2008-12-02 08:43
    I assumed the WTF was the potential for overspill. Otherwise, pretty much everything I've ever seen written in C or C-like C++ is a WTF.
    Although
    And C isn't any more about getting stuff done than anything else. C is about getting close to the actual computer behaviour. In many ways, that's counter to accomplishing a goal.
  • mortah 2008-12-02 10:22
    FIST!
  • Clank75 2008-12-02 12:06
    Yow. I have to agree with the gallery - absolutely no WTF here. Perfectly good code.


    jspenguin's heart is in the right place, but commenting to that level actually hurts readability, it doesn't help it. The original is compact, simple to understand to anyone with a passing familiarity with the language, and critically the entire function fits on the screen and is visible without scrolling.

    Adding masses of comments so you can't scan/take in the function in its entirity is not an aid to understanding.


    About the only changes I'd make would be -
    A slightly more descriptive name.
    Use #defines for the separator character and buffer length
    A *single* comment at the beginning along the lines of:
    /**************************
    
    * nice_num
    *
    * Formats a long to a string representation, with separator characters between thousands.
    * e.g. n = 123456789 is formatted to 123,456,789
    *
    * SIDE EFFECTS: Dirties the /prtbuf/ global buffer.
    * RETURNS: A pointer into /prtbuf/. Caller must copy/use before calling any other function which dirties /prtbuf/.
    */




  • John Muller 2008-12-02 23:45
    Gieron:
    Paolo G:
    bitpirate:
    The real WTF is assuming that all locales use "," for their thousands separator. "1.000.000" is common for a million in Europe.

    Yes, dot is the standard in continental Europe (which uses a comma instead of a decimal point), but the comma is used in the UK.

    In Sweden we use spaces: 1 000 000


    If that's true, I have code to fix.
  • Slicerwizard 2008-12-03 03:44
    I may have missed it, but I don't think anyone has yet posted an elegant method of handling MIN_LONG. This is my 30 year old signed integer to string code (designed for 16 bit processors):


    // converts a 32 bit integer to its string representation

    void ltoa(INT_32 number, STRING target)
    {
    INT_32 n;
    STRING ep;
    BYTE buf[12];

    if (number < 0L)
    *(target++) = '-';
    else
    number = -number;

    *(ep = buf + 10) = EOS;

    do
    {
    n = number / 10;
    *--ep = (INT_16) (n * 10 - number) + '0';
    number = n;
    }
    while (number);

    strcpy(target, ep);
    }

    I wrote a custom version of sprintf to handle separator character requirements; doing it in a modified ltoa function just gets ugly.
  • illtiz 2008-12-03 04:25
    Slicerwizard:
    I may have missed it, but I don't think anyone has yet posted an elegant method of handling MIN_LONG. This is my 30 year old signed integer to string code (designed for 16 bit processors):


    // converts a 32 bit integer to its string representation

    <snip code>


    I wrote a custom version of sprintf to handle separator character requirements; doing it in a modified ltoa function just gets ugly.


    Is it me or is this actually a bit harder to comprehend than the original snippet? What really made me spin for a bit was that you don't use modulo math (which isn't goofy). But I like how you elegantly solved the ~0 issue.
  • Rambaldi 2008-12-03 06:09
    illtiz:
    Slicerwizard:
    I may have missed it, but I don't think anyone has yet posted an elegant method of handling MIN_LONG. This is my 30 year old signed integer to string code (designed for 16 bit processors):


    // converts a 32 bit integer to its string representation

    <snip code>


    I wrote a custom version of sprintf to handle separator character requirements; doing it in a modified ltoa function just gets ugly.


    Is it me or is this actually a bit harder to comprehend than the original snippet? What really made me spin for a bit was that you don't use modulo math (which isn't goofy). But I like how you elegantly solved the ~0 issue.


    looking at it, i think it would have been slightly faster if done with a modulo operator but then not all architectures have that instruction.
  • Goran 2008-12-03 07:48
    Perhaps I've misunderstood what all the C programmers have said, but I'd gathered from other comments that C wouldn't let you type-cast then step through the string (or at least, that the available methods to do so are obnoxiously slow)? Hence the cunning use of the fact that if you add num % 10 to '0' you'll get the ascii point code from 0 - 9? And having to stack those directly into consecutive memory? Or have I got the wrong end of the stick?

    What on EARTH is it with the posters here? How do you imagine built-in funcs, operators etc in any language "typecast" a binary number to a string? They do the **cunning** remainder 10 and add to '0' 'trick'. It's the plain obvious simple mathematical way to do it. If a builtin, for some reason, isn't convenient, write it out yourself. But for heaven's sake don't go 'typecasting to string' and think something magic has happened and imagine you are avoiding doing the mathematics necessary to convert binary to decimal. And no, this isn't telling you you are dumb for not knowing a given language, this is the really basic mathematics that any programmer should know.
  • Slicerwizard 2008-12-03 13:44
    illtiz:

    Is it me or is this actually a bit harder to comprehend than the original snippet? What really made me spin for a bit was that you don't use modulo math (which isn't goofy). But I like how you elegantly solved the ~0 issue.

    Unusual solutions generally need more comments, and this code is no exception. However, they were never added way back then and since it's only a three line loop, I decided to let y'all work it out.

    Rambaldi:

    looking at it, i think it would have been slightly faster if done with a modulo operator but then not all architectures have that instruction.

    Modulo is just remainder after division. The original code has two divides per digit:

    *--buffer = '0' + (n % 10);
    n /= 10;

    Mine has one:

    n = number / 10;
    *--ep = (INT_16) (n * 10 - number) + '0';

    Divides on older hardware were very slow (~150 clock cycles on 808x) while multiplication was a bit faster, so one divide was used.
  • airdrummer 2008-12-03 14:07
    ...using apostrophes for plural's;-)
  • Ilya Ehrenburg 2008-12-03 15:15
    Slicerwizard:

    Rambaldi:

    looking at it, i think it would have been slightly faster if done with a modulo operator but then not all architectures have that instruction.

    Modulo is just remainder after division. The original code has two divides per digit:

    *--buffer = '0' + (n % 10);
    n /= 10;

    Mine has one:

    n = number / 10;
    *--ep = (INT_16) (n * 10 - number) + '0';

    Divides on older hardware were very slow (~150 clock cycles on 808x) while multiplication was a bit faster, so one divide was used.

    I think what Rambaldi meant was that using n%10 and n/= 10, a good compiler figures out that only one division is needed, because it gives you the quotient in one register and the remainder in another (if the hardware has that instruction, which I believe most modern do). I tried it, and on my box indeed it is faster to use division and modulo than one division and one multiplication, though only marginally so (~ 0.9%), with optimisation turned on. With optimisation tuned off, one division + one multiplication is significanty faster (~ 20%).
  • Slicerwizard 2008-12-04 00:28
    Gotcha. All our old compilers did for us was throw in redundant x86 register reloads all over the generated code. :(
  • foo 2008-12-04 04:45
    The code is pretty obvious actually. An it doesn't divide by 1000 because a 3-digit
    decimal number wouldn't be directly convertible to single digit's ASCII code...
    But yes, the name is silly and an explanatory comment would have been in order.
  • argh 2008-12-04 17:04
    I agree with everyone: nice_num() was a very poor choice of name for this function. Something like xtoa() would be much more readable.
  • romkyns 2008-12-05 08:33
    I looked through it briefly. It made sense. I looked through it carefully, looking for WTFs. I couldn't find any. I thought "WTF is this code doing on this site?!?!"

    And then it dawned upon me. See that WTF thought in there? That's it! Good job! :)
  • grzes 2008-12-05 09:47
    This is a simple function, it's strange that it took a few minutes to understand what it does.
  • Bo 2008-12-07 21:12
    Guys... errmm....
    I might we missing something, but why you use number as *number*, since it returns a string anyway? (Well, char in this case). If you got an input "1234567890", why it is bad to convert the thing to the string as it is and just break with commas, like "1,234,567,890" ?
  • too lazy to sign in now 2008-12-08 15:51
    Bo:
    Guys... errmm....
    I might we missing something, but why you use number as *number*, since it returns a string anyway? (Well, char in this case). If you got an input "1234567890", why it is bad to convert the thing to the string as it is and just break with commas, like "1,234,567,890" ?

    'cause string manipulation is slow, complicated and bug-prone?
    Yes, in this case it's not too complicated and the speed is probably not very important, so doing it that way wouldn't constitute a WTF either, but the implemented algorithm is more elegant.
  • wholesale jordan shoes 2008-12-22 22:37
    Air Jordan wholesale adidas shoesAir JordanAir Jordan
    Air JordanAir Jordans wholesale adidas shoes
    Air JordanAir JordanNike shoesAir Jordanair jordan nike
    Buy jordan shoesAir Jordan Air JordanAir Jordanjordan shoeAir Jordan
    air jordan nikewholesale adidas shoes
    Air Jordanretro air jordanswholesale adidas shoes
  • huojia 2009-03-02 01:12
    &nbsp;&nbsp;
    货架racking.&nbsp;&nbsp; 仓储货架
    南京货架设备要产品包括:重型货架
    ,阁楼货架,超市Shelf,重量型货架,横梁式货架,驶入式货架... 仓储笼
    货架的形式与材料 · 立体仓库 · 货架厂
    物流规划设计的步骤与程序(货架的设计... 中国物流行业呈现三足鼎立抢市场 · 亚洲第三方物流的现状和发展 ... 托盘
    抽出式货架系列 · 重力式 货架/推入式货架钢托盘
    移动式货架 · 阁楼式货架 · 悬臂式货架
    系列 长件物料储存货架&nbsp;货架公司