The Daily WTF: Curious Perversions in Information Technology

Irrelevant · 2005-02-08 Reply Admin

goto is bad in all but extreme circumstances, I agree, but break and continue can be aid clarity when used correctly, and the use documented.

For example, say I want to loop through a file one character at a time, do something to every chacter, but then only do the majority of the loop body if a condition is met:

while (c = fgetc(file)) {

        do_stuff(c);

        if (condition(c)) { /* only do the rest of the loop if foo */

                do_other_stuff(c);

                do_more_stuff(c);

/*

                It was the best of times,

                it was the worst of times...

*/

                do_the_last_stuff(c);

        } /* eek, what's this extra brace doing here? */

If you're using 8-char tabs, as above, the loop body ends up halfway across the screen, and there's an extra set of braces that doesn't fit in the logic. this is much clearer, IMO:


while (c = fgetc(file)) {

        do_stuff(c);

if (!condition(c)) continue; /* only do the rest of the loop if foo */

        do_other_stuff(c);

        do_more_stuff(c);

/*

        It was the best of times,

        it was the worst of times...

*/

        do_the_last_stuff(c);

} /* that's better */

2005-02-08 Reply Admin

Katja:
And I still disagree with the generic opinion here about the ! operator. By default, C and C++ doesn't even know any boolean types. It only recognizes if a value is zero or not. So things like !Size or !--Size are still proper usage of this operator.
This is not just my opinion. This is just the C++ Standard! I can't help it if it seems a crappy standard, but it just is. C and C++ just doesn't know any boolean types to begin with.

 The (annotated) ANSI C++
par. 3.6.1 standard specifies `bool' as a funcamental type with the
following qualities:

   1) `bool' is a unique signed integral type.
   2) A `bool' value may be converted to `int' by promotion:
      `true' converts to 1, `false' converts to 0.
   3) A numeric or pointer value may be implicitly converted to `bool'.
      A zero value becomes `false'; a non-zero value become `true'.
   4) The relational operators yield `bool' values.

2005-02-08 Reply Admin

What's up with people claiming that a state machine is the wrong solution here? It's the cleanest and simplest way to solve the problem. It's also the most efficient. What more do you want?

As for the gotos... well, it's a very short function, and after somebody finds the strtol() bug, the only time it'll ever have to be touched again will be to convert to TCHAR or Unicode, which won't affect the algorithm at all. It looks scary because it's an unfamiliar idiom, that's all. You could argue for a switch thing instead, and because that's a more popular idiom it'll be more maintainable. The switch would be wasteful, but not to a degree that's usually an issue.

There are cases where a function has to be made as efficient as humanly possible, even at some cost in readability (Moore's Law stopped applying to clock speeds two years ago, kids). In such a case this would be the Right Thing, except for that call to strtol(). If speed is really a problem, that call ought to be replaced with code that converts the hex "by hand" as another commenter suggested above.

The question is whether this optimization is premature or appropriate, and you'd really have to have seen what the profiler said before you can make that call.

The strtol() bug is a valid and serious problem. The "accept" label should (as noted above) not be called "accept", because it isn't. The eight-bit char thing is a valid WTF in this day and age; it's odd that nobody mentioned that. The objection to goto in principle is infantile. There are places (not many, but some) where it makes sense. Being a competent programmer means recognizing those places. You know, thinking -- not just robotically quacking a context-free homily some ideologue poured down your throat in CS101. God won't strike you dead if your code isn't always properly Object Oriented. OOP is a valuable tool, not an end in itself.

Bustaz Kool · 2005-02-08 Reply Admin

Katja:
And I still disagree with the generic opinion here about the ! operator. By default, C and C++ doesn't even know any boolean types. It only recognizes if a value is zero or not. So things like !Size or !--Size are still proper usage of this operator.

Minor point: C does not define 0 (zero) to be the equivalent of false. Almost all implementations of the language do make this assumption but there are a few that don't. Technically, therefore, the "!size" construct makes an inherently false assumption. C++ on the other hand does define False to be numerically zero. We used to get around the C assumption by using defines:

#define TRUE (1==1)

#define FALSE (1==0)

But that was long ago...

The overriding point about the construct for either language is that "(size == 0)" is easier for the brain to automatically digest than "(!size)".

Ben Hutchings · 2005-02-08 Reply Admin

Bustaz Kool: The comparison operators in C have always been defined as yielding an int with a value of 0 for false or 1 for true. Maybe you had to deal with broken compilers, but today those macros would be a WTF in themselves.

2005-02-08 Reply Admin

"Minor point: C does not define 0 (zero) to be the equivalent of false."

I'm not sure what planet you got your spec from, but ISO C99 is pretty clear that integer zero and logical false are interchangeable. For examples, see 6.5.3.3 paragraph 5 and 6.8.4.1 paragraph 2. Are you confusing this with the issue where null pointers may have non-zero bits? (Note that "(!nullpointer)" must still evaluate to true for any null pointer, but you cannot portably rely on memset(&ptr, 0, sizeof(ptr)) to make ptr a null pointer.)

The overriding point about the construct for either language is that VB (or Pascal) programmers should stick to VB (or Pascal) and not attempt to make other languages look like VB (or Pascal). I have known people to #define begin { and #define end ;}, but that does not make it idiomatic C.

If that was an attempt at humor, perhaps you should trade your sense of humor in for a new one.

2005-02-08 Reply Admin

:
Some developers cannot be given any powerful language. It's just like giving a gun to a chimpanzee. You'd be lucky if the only thing they shoot is their own feet.

Why are you being so mean to me?

JamesCurran · 2005-02-08 Reply Admin

Katja:
And I still disagree with the generic opinion here about the ! operator. By default, C and C++ doesn't even know any boolean types.

As others have noted, C++ does in fact have a native boolean type. "true" and "false" are even keywords.

So things like !Size or !--Size are still proper usage of this operator.This is not just my opinion. This is just the C++ Standard!

No, it's no. And don't turn to the C++ Standard as a defense. It only reports on what will compile. Not on what are good programming practices.

Remember, our goal here is not merely to write code that will compile, but code that other people can read & understand -- preferable without having to think too much about it.

So, if you were to take an int an use it as a boolean:
int MyFlag = TRUE;

Then I would have no problem using a !:
if (!MyFlag)

However, if you are going to use an int as a number, then you should use operators that relate too it as a number:
int MyCounter = 15;
if (MyCounter == 0)

JamesCurran · 2005-02-08 Reply Admin

:
What we weren't allowed was something like this:
for(int i = 0; i < somecount; i++){
//blablabla code here
if(someboolistrue){
break;
}
}

But he said nothing about your not aligning the opening & closing braces?

So, he'd want you to write (using a real example)

for(int i = 0; i < somecount; i++)
{
if(a[i] == target)
break;
}
return i;

as :

int foundit;
for(int i = 0; i < somecount; i++)
{
 if(a[i] == target)
 {
 foundit = i;
 i = somecount;
 }
}
return foundit;

Ick!

IckIckIckIckIck!!!

So, let's see.... It's Ugly, Confusing, and requries the processor to do more work. It's a three time loser!

AK-47 · 2005-02-08 Reply Admin

JamesCurran:
Actually, most of the computer teachers I know, do consulting jobs over the summer.

I think this site could open special corner dedicated to code/solutions written by consultants. I don't want to blame all academy folks, but experience in software production is quite rare knowledge among them. Usually they have pompous titles and prefixes on their business card, but little understanding what the real problems of software development are. It's because computer sciences and software development are two different things (however, with some common basis). Unfortunately, many managers think that CS degree automatically bestows SW development skills and experience. They mistaken bitterly.<?xml:namespace prefix = o ns = "urn:schemas-microsoft-com:office:office" /><o:p></o:p>

Any SW developer (with degree or without one) who touched real code just once knows that there are no silver bullets and universal rules. There is no such thing in real life as "if you'll use goto consider yourself fired". There are problems thatrequire reasonable, maintainable and comprehensible solutions.

AK-47 · 2005-02-08 Reply Admin

to learn how to think and how to do smart decisions:
idd and mine is, not to use break/goto/continue stuff Smile
The fact that you do is, off course your business.

There are no all embracing universal rules like that. The C++ language contains these keywords and there are situations when you'll use them. If you don't believe me, then take any big C++ library and grep for break/goto/continue.

2005-02-08 Reply Admin

jamescurran, you're on hop about the counters.

Treating a counter as a boolean is absolutely canonical C/C++ idiom and has been so since the 1970s. Any competent programmer knows what it means and recognizes it instantly. If it weren't so pervasive, I'd agree with you, but it is, so don't worry about it.

And just by the way, more people should do the constant-on-the-left thing. Like so:

if ( 0 == counter ) { ... }

That's so if you should happen to mistype the operator, the compiler will tell you. It's a good habit. Won't save you if you're comparing two valid lvalues, of course.

2005-02-08 Reply Admin

I do see a certain point in disallowing break from loops, though it's not a general rule. Often, break can simplify the code, but it can also be confusing, especially when the loop body grows large.

As for the above Code, That would be best written as

for(int i = 0; i < somecount && a[i] != target; ++i);
return i;

2005-02-08 Reply Admin

FB MVP C#:
rubbish like "only one returnpoint per function!" or "goto is evil!" or "if a function is longer than a screen, create 2 functions!"

Doh! Typical comment... Apparently (code) readability and maintenance are not in your vocabulary... Too bad... :-(

W.

JamesCurran · 2005-02-08 Reply Admin

AK-47:
Any SW developer (with degree or without one) who touched real code just once knows that there are no silver bullets and universal rules.

Ya'know... I used to feel exactly the same way --- for the first 3 or 4 years after college, in the "real world". Then I slowly came to realize the saying "this is 'real world' code--- the rules don't apply" was just a way of saying "I'm too lazy to do it right", and that the more I followed those rules they taught me in college, the better my code was.

JamesCurran · 2005-02-08 Reply Admin

:
jamescurran, you're on hop about the counters.

Treating a counter as a boolean is absolutely canonical C/C++ idiom and has been so since the 1970s. Any competent programmer knows what it means and recognizes it instantly.

Define "instantly". I'd conjecture that my way recognition takes 1/10 of a second, while your way takes 1/2 a second. You probably don't notice it at first, but across a page, it adds up. I thought I would recognize those idioms just as fast as the correct way, until I stopped using them and almost wrote the explicit form, and I soon realize how faster & easier it was to understand the code.

if ( 0 == counter ) { ... }

Well, I agree in principle, but it never helped me in practice. The problem is, if you can remember to write the constant first, you can remember to use two ==s. The one place where it is useful, is when the non-constant is a function call with a long parameter list:

if (MB_YES == MessageBox("An Error Occurred. Shall I continue?", MB_YESNO))

That way it keeps all the important points ("if", "MB_YES", "MessageBox") together, so you don't have to scan the entire line to know what's going on.

JamesCurran · 2005-02-08 Reply Admin

:
As for the above Code, That would be best written as
for(int i = 0; i < somecount && a[i] != target; ++i);
return i;

"Best"? I'd like to hear your explanation why it's even "better" than mine.

Downside of yours: It's confusing: If you don't notice the semicolon at the end of the for() statement, you'll greatly misinterpret what it's doing. (And, of course, if you forget the semi-colon weird things happen).

It's also an abuse of the for() statement. (if you'd used a while statement, I'd object less)

int i =0;
while ( i < somecount && a[i] != target)
++i;

The downside there is that it take attention away from the important part (a[i]!=target) and places it on a triviality (++i).

Note that each version generates object code which is byte-for-byte identical (Visual C++ 2003, Release build)

2005-02-08 Reply Admin

With the performance improvements suggested by JamesCurran and the bug fix noted by Ben Hutchings incorporated here is the final (hopefully) code.

/* 
 * Converts a string with embedded escape sequences to a plain string.
 * eg. "C%23" becomes "C#", "%24505" becomes "$505".
 */
h2a_err_t hex2asc( char * to, const char * from, size_t size )
{
    size_t idx_src   = 0;
    size_t idx_dest  = 0;
    h2a_err_t rtn    = H2AERR_OK;
    char hex_str[3]  = { 0 };

    if (0 == size)
    {
        return H2AERR_BUFFER_TOO_SMALL;
    }

    size--;

    while ( from[idx_src] != '\0' )
    {
        if (idx_dest >= size)
        {
            rtn = H2AERR_BUFFER_TOO_SMALL;
            break;
        }

        if (from[idx_src] != '%')
        {
            to[idx_dest++] = from[idx_src++];
        }
        else
        {
            if (!isxdigit( (unsigned char) from[idx_src + 1]) ||
                !isxdigit( (unsigned char) from[idx_src + 2]))
            {
                rtn = H2AERR_INVALID_HEX;
                break;
            }

            hex_str[0] = from[idx_src + 1];
            hex_str[1] = from[idx_src + 2];

            to[idx_dest++] = (char) strtol(hex_str, NULL, 16);

            idx_src += 3;
        }
    }

    to[idx_dest] = '\0';

    return rtn;
}

For those who object to

the use of goto/break/continue, I'd be interested in seeing how you would rewrite this code without the use of the break keyword.

While the original code contains a couple of bugs and is unreadable it did have the following positive aspects.

Correct behaviour if size is 0.
Guarantees nul termination of the destination buffer if size is greater than zero.
Meaningful return values collected in an enum.

AK-47 · 2005-02-08 Reply Admin

JamesCurran:
Then I slowly came to realize the saying "this is 'real world' code--- the rules don't apply" was just a way of saying "I'm too lazy to do it right", and that the more I followed those rules they taught me in college, the better my code was.

No, no, no. Under no circumstances I would say that "real code" = "sloppy code". My point is that statements like "never use that operator" and "never use this keyword" are inherently wrong. They're wrong due to one simple fact: we can't predict the future. No matter how infrequent some language feature is there can be situation when using this unpopular feature is "the right thing to do".

This is what makes difference between novice and experienced mature developer. Experienced one will never do absolute statements because he knows well that reality is too diverse. We all have our rules of thumb, but we don't follow them blindly, but rather question those rules each time we use them.

Drak · 2005-02-09 Reply Admin

/* 
 * Converts a string with embedded escape sequences to a plain string.
 * eg. "C%23" becomes "C#", "%24505" becomes "$505".
 */
h2a_err_t hex2asc( char * to, const char * from, size_t size )
{
    size_t idx_src   = 0;
    size_t idx_dest  = 0;
    h2a_err_t rtn    = H2AERR_OK;
    char hex_str[3]  = { 0 };

    if (0 == size)
    {
        return H2AERR_BUFFER_TOO_SMALL;
    }

    size--;

    while ( from[idx_src] != '\0' )
    {
        if (idx_dest >= size)
        {
            to[idx_dest] = '\0';



            return H2AERR_BUFFER_TOO_SMALL;

        }

        if (from[idx_src] != '%')
        {
            to[idx_dest++] = from[idx_src++];
        }
        else
        {
            if (!isxdigit( (unsigned char) from[idx_src + 1]) ||
                !isxdigit( (unsigned char) from[idx_src + 2]))
            { 
                to[idx_dest] = '\0';

                return H2AERR_INVALID_HEX;     
            }

            hex_str[0] = from[idx_src + 1];
            hex_str[1] = from[idx_src + 2];

            to[idx_dest++] = (char) strtol(hex_str, NULL, 16);

            idx_src += 3;
        }
    }

    to[idx_dest] = '\0';

    return rtn;
}

:
For those who object to the use of goto/break/continue, I'd be interested in seeing how you would rewrite this code without the use of the break keyword.

See above..

Drak

--
The wheel should be reinvented until it rolls smoothly, is easy to build, and requires no maintenance.

2005-02-09 Reply Admin

JamesCurran:
This code is placed in the public domain.

pet peeve.... Never place something in the "public domain". You are giving up more rights than you think. Better to just say "You may use it freely".

No, you are giving up a very specific and limited set of rights. You are giving up your exclusive rights to copy, modify, distribute, prepare derivitive works of, and publicly display/perform the work in question. There is no reason not to do this for code you don't want to keep under your exclusive control. Doing something useless like saying "you may use it freely" causes the internet to be full of dead code that people can't use for fear of later lawsuits. If you don't wish to maintain exclusive rights to a work, making it public domain is exactly what you should do. Otherwise, you need to release it under a legally binding license, or nobody can use it at all.

JamesCurran · 2005-02-09 Reply Admin

Drak:

/* 
 * Converts a string with embedded escape sequences to a plain string.
 * eg. "C%23" becomes "C#", "%24505" becomes "$505".
 */

See above..

But to accomplish that, you had to break the "one return rule" and with it duplicate the closing code (which is why there is a 1RR) three times. One the whole, a far may heinous crime.

2005-02-09 Reply Admin

When adherence to stuff like the "one return rule" leads you into writing code that's harder to read and/or maintain than it has to be, it's bad. I have seen people tie themselves into absurd knots trying to stick to one arbitrary dogma or another, and it's nonsense. Yes, a sufficiently ingenious programmer can get things done with all manner of inappropriate constructs, but that's not to say he should.

The prohibition on break in loops is the work of an imbecile. There's no way to be tactful about that one. The moron who dreamed it up probably thought break should be banned because it resembles goto, but that's insane: Sure, there's a jump in both, but all flow-of-control constructs involve jumps.

Programming is like writing or cooking or driving a car: Learn the rules, learn to think for yourself, and when the time comes to break the rules, be sure you know what you're doing. I wouldn't want a college freshman running around loose with goto, or automatic weapons, or powerful explosives. But I'd hate to have to try to get by without those tools myself.

JamesCurran, until constant-on-the-left becomes a habit, it helps focus your mind on the expression; after it becomes a habit, you won't be paying close attention again and you'll want the warning.

You're absolutely right about the MB_YES thing, of course. Much more clear that way. MessageBox() would be my number-one example, too.

2005-02-09 Reply Admin

"People who started with C and then latter pick up a little C++ are considered dangerous."

Isn't Bjarne one of these bastards? [:P][<:o)]

2005-02-10 Reply Admin

if (!strcpy(str,"ABC"))
Now, I think most people when seeing that, would at first read it as "If str is NOT equal to 'ABC' " (when it in fact means "If str IS equal to 'ABC' "

Why would they think that: strcpy returns a pointer to str, so that should never be 0.

Maybe you were thinking of strcmp(), which returns a useful if slightly unintuitive value... The C standard libraries do that a lot (esp. the functions that return 0 on success something else on error - it's mostly to do with the absence of exceptions, multiple return values and garbage collection). That doesn't make it a WTF to use ! on the value returned by such a function - it just means that C can be very unintuitive because of its low-level design.

Note to other people balking at goto() - since C has no exceptions and garbage collection, using goto()s in C can be a lot cleaner than the alternatives.

2005-02-10 Reply Admin

I like to call code like what you are describing from that crowd as "C+"

2005-02-11 Reply Admin

JamesCurran:
[image] Katja wrote:

Glad I'm learning C++ for school right now else I'd believe you're right. But no... In C++ a boolean is either zero or non-zero, no matter what type it is. So (!size) is still correct usage of the NOT operator. Yes, it seems funny but C++ is a funny language.

It is definitely not abuse of the ! operator because the C++ standard declares this as legal usage.

Just because something compiles doesn't mean it's right. C (and therefore C++, due to backward compatibility), will allow you to get away with lots of ugliness --- but that doesn't mean that you should do it.

I think the worst use of it is in a line like:

if (!strcpy(str,"ABC"))

Now, I think most people when seeing that, would at first read it as "If str is NOT equal to 'ABC' " (when it in fact means "If str IS equal to 'ABC' ")

Using ! on non-bools is a bad practice --- Remember, if someone is too lazy to type TWO EXTRA CHARACTERS, just think about what other "shortcuts" they might have done....

AndrewB · 2005-02-12 Reply Admin

c = *from++;

I see this one scattered throughout the code. It seems like he's trying to "walk the pointer" and dereference it. But then shouldn't he doing this?

c = *(from++);

Because, after all, isn't this the default effect?

c = (*from)++;

I haven't really read much into the code. Maybe he is trying to do a variable increment and not a pointer-walk. Can anyone shed some light on this?

2005-02-23 Reply Admin

No, actually, it does the same as:

c = *(from++)

Postincrement has a higher precedence than dereference. It's actually a somewhat common idiom.

2005-02-25 Reply Admin

Isn't this point moot?

Doesn't

c = (*from)++; put the same value into "c" as:

c = *(from++)

?

2005-02-25 Reply Admin

Re: my previous post. Re: (*from)++ vs. *(from++)

Doh!

Hope everyone enjoyed my extra WTF.

Schol-R-LEA · 2005-02-26 Reply Admin

The issues of gotos, bools, and so forth are secondary to the real problem with this: the obvious solution of converting the hex value to an int, then convert the int to the needed decimal string, is both simpler and faster than trying to perform some kind of textual transform for what is, in point of fact, a representation of a numeric value. This can be done in two lines of C code using standard library functions, and given the essential efficiency of the basic algorithmss in question, would probably be faster than any textual transform by an order of magnitude.

2005-03-01 Reply Admin

I agree, Irrelevant. Breaks are also very good for nested loops in my experience, and they get a bad wrap.

x = rs.getNextRecord()

while(x)

{
 fieldVal = x->field1;
 echo '<h1>' . fieldVal . '</h1>';
 while(fieldVal == x.field1)
 {
 fieldVal2 = x->field2;
 echo '<h2>' . fieldVal2 . '</h2>';
 
 while(fieldVal2 == x->field2 && fieldVal == x->field1)
 {
 doStuff(x);
 doInnerEndStuff();
 x = rs.getNextRecord();
 if(!x) break;
 }
 doSecondEndStuff();
 if(!x) break;
 }
 doOuterEndStuff();
}

phx · 2005-03-04 Reply Admin

In that case I would feel justified using a goto.

If Not Done makes me ill :P

2009-05-04 Reply Admin

Katja:
It is definitely not abuse of the ! operator because the C++ standard declares this as legal usage. So you're wrong! Nananaanaaaana! [:P]

It's legal but not legible.

If size were used as a boolean variable here, applying a boolean operator like '!' would be fine. Instead, size holds the number of characters (in *to, presumably).

So size is used as a number, and comparison should be used, e.g. size != 0.

Welcome to the next level of understanding.

2009-05-04 Reply Admin

:
And just by the way, more people should do the constant-on-the-left thing. Like so:
if ( 0 == counter ) { ... }

Except that once you start with C++, derive your own classes and write operator==(), interesting things begin to (not) happen...

In C++, more people should do the const keyword thing.

2009-05-04 Reply Admin

:
if (!strcpy(str,"ABC"))Now, I think most people when seeing that, would at first read it as "If str is NOT equal to 'ABC' " (when it in fact means "If str IS equal to 'ABC' "Why would they think that: strcpy returns a pointer to str, so that should never be 0. Maybe you were thinking of strcmp(), which returns a useful if slightly unintuitive value... The C standard libraries do that a lot (esp. the functions that return 0 on success something else on error - it's mostly to do with the absence of exceptions, multiple return values and garbage collection). That doesn't make it a WTF to use ! on the value returned by such a function - it just means that C can be very unintuitive because of its low-level design.
Note to other people balking at goto() - since C has no exceptions and garbage collection, using goto()s in C can be a lot cleaner than the alternatives.

Nobody forces you to write non-intuitive things as long as there are legal alternatives. strcmp() becomes quite intuitive when you compare its result value against 0. That's how it was meant to be used; unfortunately the man page doesn't explain that.

Of course, that applies only if you write the 0 constant last, as one does naturally (i.e. as one learned in maths).

Yet Another "Hex To Ascii"

Leave a comment on “Yet Another "Hex To Ascii" ”

Yet Another &quot;Hex To Ascii&quot;

Leave a comment on “Yet Another &quot;Hex To Ascii&quot; ”

Yet Another "Hex To Ascii"

Leave a comment on “Yet Another "Hex To Ascii" ”