Comment On Fred Code

Articles like this make me think:


IF ONLY I, TOO, WERE PAID BY THE LINE !

I like how the code draws a huge arrow. Also, this code shows that Fred needs to be beat senseless with a copy of the printf(3) man page... at least he didn't create some monstrosity that would result in #include <cobol.h> though!

You can imagine Fred muttering "Stupid C, why doesn't it have PICs" as he wrote this !

But that piece of code looks beautiful, it's almost poetry!

kastein:

I like how the code draws a huge arrow. Also, this code shows that Fred needs to be beat senseless with a copy of the printf(3) man page... at least he didn't create some monstrosity that would result in #include <cobol.h> though!

Yes, the arrow is good. Too bad nobody shot Fred with it...

Nine character fixed width columns, hm. I say Fred was innocent of being a COBOL programmer; he was worse.

He was a FORTRAN programmer.

Let's see... the <= from 0 onward should be < instead. There also seem to be some missing commas near the end. I won't try to find the other bugs, unless I actually want to get a headache.

kastein:

Fred needs to be beat senseless with a copy of the printf(3) man page

$ man 3 printf | wc -l
343

That wouldn't be thick enough to do any damage. Better make it a copy of _The C Programming Language_, preferably in hardcover.

void ConvertLongToString(char NinecharString[], long Number)

{

	if (Number <= -10000000)

		sprintf(NinecharString, "%ld", Number);

	else if (Number <= -1000000)

		sprintf(NinecharString, "0%ld", Number);

	else if (Number <= -100000)

		sprintf(NinecharString, "00%ld", Number);

	else if (Number <= -10000)

		sprintf(NinecharString, "000%ld", Number);

	else if (Number <= -1000)

		sprintf(NinecharString, "0000%ld", Number);

	else if (Number <= -100)

		sprintf(NinecharString, "00000%ld", Number);

	else if (Number <= -10)

		sprintf(NinecharString, "000000%ld", Number);

	else if (Number <= 0)

		sprintf(NinecharString, "0000000%ld", Number);

	else if (Number <= 10)

		sprintf(NinecharString, "00000000%ld", Number);

	else if (Number <= 100)

		sprintf(NinecharString, "0000000%ld", Number);

	else if (Number <= 1000)

		sprintf(NinecharString, "000000%ld", Number);

	else if (Number <= 10000)

		sprintf(NinecharString, "00000%ld", Number);

	else if (Number <= 100000)

		sprintf(NinecharString, "0000%ld", Number);

	else if (Number <= 1000000)

		sprintf(NinecharString, "000%ld", Number);

	else if (Number <= 10000000)

		sprintf(NinecharString, "00%ld", Number);

	else if (Number <= 100000000)

		sprintf(NinecharString, "0%ld", Number);

	else if (Number <= 1000000000)

		sprintf(NinecharString, "%ld", Number);

	else

		sprintf(NinecharString, "000000000");

}

fix'd

Fabricated story?
Possibly - specifying "char NinecharString[]" instead of "char[] NinecharString" yields a compilation error in my world at least.

And what about those "embedded minus signs"? Where?

And why would passing a zero to the function yield nulls? There's a "<= 0" test right there...

The true WTF is php.

Compilers which don't emit errors when two function names longer than 32 characters clash are an additional WTF.

Perfect coder:

And what about those "embedded minus signs"? Where?

E.g. 00000-150

Perfect coder:

And why would passing a zero to the function yield nulls? There's a "<= 0" test right there...

It prints an 8-character string followed by a \0.

On reflection it occurs to me that there's probably an additional bug - how many of the places which call it give a 9-char buffer which will be overflowed by the null terminator?

Perfect coder AKA Barney:

Fabricated story?
Possibly - specifying "char NinecharString[]" instead of "char[] NinecharString" yields a compilation error in my world at least.

And what about those "embedded minus signs"? Where?

And why would passing a zero to the function yield nulls? There's a "<= 0" test right there...

Hey! Fred's neighbor and best bud Barney is a programmer too! Who'd'a thunk it?

Perfect Coder:

Fabricated story?
Possibly - specifying "char NinecharString[]" instead of "char[] NinecharString" yields a compilation error in my world at least.

It seems your world does not intersect with the world in which Microsoft Visual Studio exists.

Perfect coder:

Fabricated story?
Possibly - specifying "char NinecharString[]" instead of "char[] NinecharString" yields a compilation error in my world at least.

Then your world is not the world of C as was clearly stated in the article.

Month number, sales tax percentage, weight in pounds, you name in.

My name in what? Come on, the suspense is killing me!

Edit: I get it! My name in a nine char string!

Perfect coder:

Fabricated story?
Possibly - specifying "char NinecharString[]" instead of "char[] NinecharString" yields a compilation error in my world at least.

This is before arrays were objects. Arrays began as sizeof(type) * len sequential bytes and the only concept of "null" was a pointer set to 0. In many ways, I prefer C and C++ to C#. Of course, in some ways the opposite can be said as well.

I would have liked to see the fix. :P It's most fun picking apart the "fixed" code. :P

the char[] NineCharstring statement is C++ specific... unless recent C standards have back ported it from C++

Jack Hughes:

the char[] NineCharstring statement is C++ specific... unless recent C standards have back ported it from C++

Funny, I've never seen that syntax in C++...

>ConvertLongToString(char NinecharString[], long Number)

The posting makes fun of the long names. Would it be better if this were called "cls" or "cls9"?

Certainly there's a lot better way to write the code. But I thought the comment on long function names to be a cheap shot. Having done some maintenance programming pouring through listings, seeing a meaningful identifier is a blessing, rather than having to go external to the routine I'm studying to figure out what 'cls9" does.

Yanman:

The true WTF is php.

Are you a troll, or just commenting on the wrong article or something?

Also, while the code is awful, it does look beautiful. If you make the font size really small, then turn your head on one side and squint, it looks like the Delta Airlines logo (and you'll look really silly). Perhaps The Daily WTF has started accepting subliminal advertising?

JoLoCo:

Yanman:

The true WTF is php.

Are you a troll, or just commenting on the wrong article or something?

Also, while the code is awful, it does look beautiful. If you make the font size really small, then turn your head on one side and squint, it looks like the Delta Airlines logo (and you'll look really silly). Perhaps The Daily WTF has started accepting subliminal advertising?

I tried to troll, but then I realised I was speaking the truth.

You know what I've always liked about a lot of the CodeSODs?

The fact that they (usually) make symmetrical or interesting patterns and in some cases can double for those Magic Eye pictures that were big in the 90s.

Perhaps that's a new determining factor of potentially bad code? Is it symmetrical/does it form some kind of visual pattern or not?

So his solution didn't include the risky:

sprintf(buf,"%09ld",n);

I'd love to see what he wrote!

The char name[] syntax is C and C++, and precisely equal to char * name, just slighly more confusing.

GWLlosa:

Articles like this make me think:


IF ONLY I, TOO, WERE PAID BY THE LINE !

You mean you write code like the guy in the article.
WTF
Shame on you :P

$ man 3 printf | wc -l
343

That wouldn't be thick enough to do any damage. Better make it a copy of _The C Programming Language_, preferably in hardcover.

Depends on the font. Where here's a will, there's a 384-point way.

Seth Jeacopello:

You know what I've always liked about a lot of the CodeSODs?

The fact that they (usually) make symmetrical or interesting patterns and in some cases can double for those Magic Eye pictures that were big in the 90s.

Perhaps that's a new determining factor of potentially bad code? Is it symmetrical/does it form some kind of visual pattern or not?

Then you need this.

Are those negative numbers going to come out looking like 00-12345678

GWLlosa:

Articles like this make me think:

IF ONLY I, TOO, WERE PAID BY THE LINE !

/*

 * Name:  char *Comment(char *Name,char *Poster,int MaxLen,char *Buffer)

 *

 * Purpose:  Generate a comment

 *

 * Parameters:  char *Name -- Name of comment.

 *              char *Poster -- Name of the one posting the comment.

 *              int MaxLen -- Maximum length of result.

 *              char *Buffer -- Buffer to store comment.

 *

 * Returns:  Pointer to buffer.

 *

 * Notes:  I

 *          get

 *           paid

 *            by

 *             the

 *              line!

 *         I

 *          have

 *           the

 *            best

 *             documented

 *              code

 *               around!

 */

extern

  char *

    Comment(

            char *Name,

            char *Poster,

            int MaxLen,

            char *Buffer

            );

Surprised no one else found the other bug yet.

HINT: It has something to do with formatting 10 with the same number of leading zeros as (and formatting 100 with the same number of leading zeros of as 99 (and formatting 1000 with same same number of leading zeros as 999 ( *add additional recursive comments here*)))

Tundog:

Surprised no one else found the other bug yet.

You're late by an hour and a half... See #7:

Captain Oblivious:

Let's see... the <= from 0 onward should be < instead. There also seem to be some missing commas near the end. I won't try to find the other bugs, unless I actually want to get a headache.

Tundog:

Surprised no one else found the other bug yet.

HINT: It has something to do with formatting 10 with the same number of leading zeros as (and formatting 100 with the same number of leading zeros of as 99 (and formatting 1000 with same same number of leading zeros as 999 ( *add additional recursive comments here*)))

Maybe http://thedailywtf.com/Comments/Fred-Code.aspx#258426?

pjt33:

Compilers which don't emit errors when two function names longer than 32 characters clash are an additional WTF.

That would be a neat trick, since it's the linker that has to detect that two identical 32-character strings from the compiler aren't actually referring to the same thing.

pjt33:

On reflection it occurs to me that there's probably an additional bug - how many of the places which call it give a 9-char buffer which will be overflowed by the null terminator?

Probably none. People who write this kind of code live in a universe of fixed-size strings.

I'd imagine the code is used like this:

char oneThirtyTwoColumnLine[132];



memset(oneThirtyTwoColumnLine, ' ', 132);



sprintf(&oneThirtyTwoColumnLine[1 - 1], "%s", name);



// Fix bug #10169

oneThirtyTwoColumnLine[strlen(oneThirtyTwoColumnLine)] = ' ';



ConvertLongToString(&oneThirtyTwoColumnLine[16 - 1], age);

ConvertLongToString(&oneThirtyTwoColumnLine[25 - 1], height);

ConvertLongToString(&oneThirtyTwoColumnLine[34 - 1], weight);

// ...

ConvertLongToString(&oneThirtyTwoColumnLine[71 - 1], genderPreferences);

printf("%s\n", oneThirtyTwoColumnLine);

In other words, the null at the end just gets overwritten by the first character of the next field, except for the null after the last field which is the end of the line anyway.

vt_mruhlin:

Are those negative numbers going to come out looking like 00-12345678

No -12345678 would be just that (formatted to nine characters). -1234567 would print formatted as -012345678

printf ("%.9d\n",1);

You know, any sensible COBOL programmer would use a library function, eg: printf or one of its equivalents - have you ever programmed in COBOL?

In fact, I strongly doubt a COBOL programmers would have ever written such a piece of rubbish...

However, like many others I'm constantly surprised by the degree of idiocy I see...

...and if this guy was ever a COBOL programmer, then he never got his hands on any real projects...

snoofle:

So his solution didn't include the risky:

sprintf(buf,"%09ld",n);

I'd love to see what he wrote!

Risky indeed! Sprintf only guarantees minimum size, not maximum. You really want snprintf(buf, 9, "%09ld", n). This way instead of memory errors, you'll just get truncated digits. You can even check for truncations by looking at snprintf's return value! (if return value > 9)

BUT IT LOOKS SO PRETTY!

Seth Jeacopello:

You know what I've always liked about a lot of the CodeSODs?

The fact that they (usually) make symmetrical or interesting patterns and in some cases can double for those Magic Eye pictures that were big in the 90s.

Perhaps that's a new determining factor of potentially bad code? Is it symmetrical/does it form some kind of visual pattern or not?

Go to http://www.ioccc.org/years-spoiler.html
Search for Herrmann2.

Zork II:

You know, any sensible COBOL programmer would use a library function, eg: printf or one of its equivalents - have you ever programmed in COBOL?

In fact, I strongly doubt a COBOL programmers would have ever written such a piece of rubbish...

However, like many others I'm constantly surprised by the degree of idiocy I see...

...and if this guy was ever a COBOL programmer, then he never got his hands on any real projects...

I must concur. The fact is, former COBOL programmers should not be looked down upon by all the "C" programmers adding their comments here. Different times and different system requirements. Sort of like people making derisive remarks about php programmers.

As yanman remarked...it is 'pretty', just not 'smart'. Hey, if pretty DID = smart then Hollywood would have an IQ over 42.

Zygo:

I'd imagine the code is used like this:

[...]

printf("%s\n", oneThirtyTwoColumnLine);

I don't think that's it. If the output of CLTS gets treated as a C-style string, then his embedded NULL bug will result in output truncation. Since the problem is embedded NULLs in the output file, he has to be treating it as an array of individual characters and assuming it's already the proper length.

So it might look something like:

for (i=0; i<9; ++i) {

  fprintf(fp, "%c", CltsOutputArray[i]);

}

camelCase

NotCamelCase

A decent COBOL programmer would have saved the file with some sort of separator between the numbers (eg commas to create a traditional CSV file).

Then they'd read the lines back and use COBOL's equivalent of PARSE (the COBOL verb is UNSTRING) to collect the individual values into an array.

Simplicity itself. The guy clearly did not know how to do a simple task in *any* language.

I'm worried about this line in the article.

Not daring to do the sensible, one-line sprintf()-based solution, Bruce instead wrote a replacement that behaved exactly the same way.

Does this say that Bruce wrote a routine that did the same thing as Fred's routine?
Why?

Pim:

I'm worried about this line in the article.

Not daring to do the sensible, one-line sprintf()-based solution, Bruce instead wrote a replacement that behaved exactly the same way.

Does this say that Bruce wrote a routine that did the same thing as Fred's routine?
Why?

It is obvious...
...Bruce has now become a 'consultant' and is paid by the line.

Who said Fred couldn't teach him anything?

Pim:

Does this say that Bruce wrote a routine that did the same thing as Fred's routine?
Why?

Fred's routine would render -10 as "000000-10" instead of "-00000010". Bruce kept that behavior just in case other code was dependent on it.

Erasmus Darwin:

Zygo:

I'd imagine the code is used like this:

[...]

printf("%s\n", oneThirtyTwoColumnLine);

I don't think that's it. If the output of CLTS gets treated as a C-style string, then his embedded NULL bug will result in output truncation.

Yeah, I buggered up my own example at the last line. Call it memory loss due to PTSD due to having to work with that kind of crap... ;)

Maybe something like:

fwrite(oneThirtyTwoColumnLine, 1, 80, // We only use 80 columns anyway

       outputFile);

fputc('\n', outputFile);

although the examples I'm thinking of actually used writev(2).