The Daily WTF: Curious Perversions in Information Technology

2010-01-27 Reply Admin

I think that's great. I'd remove the extraneous {}'s and have an efficient enough solution in 3 lines.

joelkatz · 2010-01-28 Reply Admin

Heron:
joelkatz:
I sometimes wish C had a way to name a scope and 'break' or 'continue' a named scope.

'goto' will do exactly that. It's silly to add a feature to the language that does not reduce verbosity merely because you want to avoid using the word 'goto'.

...

How would "named scopes" be less verbose?

Well, they would be slightly less verbose. The syntax could be something like:

for(...) <outer>
{
 for(...) <inner>
 {
  if(...) continue inner;
  ...
  if(...) break outer;
 }
}

And that would eliminate the need to maintain the goto labels. It would also permit a clever editor to identify, (by name), what scope closing braces or 'while' clauses were associated with.

But your point is definitely valid. Named scopes really are a very slightly more elegant way of providing the same capability as 'goto'.

2010-01-28 Reply Admin

IMHO, this:

Cbuttius:
Although you might have to call ToString(), you only need to call it once on ID - that never changes, so unless it's pretty much a no-op (i.e. a Get function) call it before the loop.

plus this:

ShatteredArm:

Also, I thought of a more elegant solution:

bool doContinue = false;
for (int i = 0; doContinue && i < dtModules.Rows.Count; i++)
    {
        if (dtModules.Rows[i]["id"].ToString() == ID.ToString())
        {
            dtModules.Rows.RemoveAt(i);
            doContinue = false;  //used to jump out of the for
        }
    }

= the answer

2010-01-28 Reply Admin

John:
IMHO, this:
Cbuttius:
Although you might have to call ToString(), you only need to call it once on ID - that never changes, so unless it's pretty much a no-op (i.e. a Get function) call it before the loop.
plus this:
ShatteredArm:
Also, I thought of a more elegant solution:
bool doContinue = false;
for (int i = 0; doContinue && i < dtModules.Rows.Count; i++)
    {
        if (dtModules.Rows[i]["id"].ToString() == ID.ToString())
        {
            dtModules.Rows.RemoveAt(i);
            doContinue = false;  //used to jump out of the for
        }
    }
= the answer

I should add, with the fix for the first line -

bool doContinue = true;

Helix · 2010-01-28 Reply Admin

TRWTF is that he did not use break

2010-01-28 Reply Admin

Apparently the origional coder don't think it's worthWHILE to use WHILE loop.

2010-01-28 Reply Admin

Too verbose. How about

int i = 0; while(((dtModules.Rows[i]["id"].ToString() != ID.ToString()) || 34598 == dtModules.Rows.RemoveAt(i)) && ( dtModules.Rows.Count > ++i) );

This assumes that dtModules.Rows.RemoveAt(i) returns an integer != 34598; if return type of dtModules differs, code must be adjusted accordingly.

2010-01-28 Reply Admin

Clearly this coders boss does a project wide search for such evil constructs as break, and goto, and absolutely forbids them. The coder disagreed with it and decided to protest by leaving a randomly failing piece of code.

2010-01-28 Reply Admin

The comment after the assign of i to 34598 is the real WTF.

The algorithm is to remove the first matching record in the first 34598 records, and then any matching records after that point.

2010-01-28 Reply Admin

It's not a clean quote, but since it is my own old post:

Thorsten M.:
#define C1 /* second expression is only evaluated if first is false */
#define C2 /* This assumes dtModules.Rows.RemoveAt has return type int and does not return 34598 most of the time */
int i = 0;
while(  (   (dtModules.Rows[i]["id"].ToString()!= ID.ToString()) 
         || 34598 == dtModules.Rows.RemoveAt(i)) C1 C2
      &&( dtModules.Rows.Count > ++i) );
This assumes that dtModules.Rows.RemoveAt(i) returns an integer != 34598; if return type of dtModules differs, code must be adjusted accordingly.

In case dtModules.Rows.RemoveAt(i) doesn't have a return value, a wrapper

int wrapper(int i) { dtModules.Rows.RemoveAt(i); return -1; }

is required, extending the code to three lines. This could be used anyway to avoid the bother to lookup the return type, but it's less fun: The thrill of the possibility that the funtion might actually return 34598 and thus lead to more interesting behaviour would be missed. Then again

int wrapper(int i) { dtModules.Rows.RemoveAt(i); return ( 60000*random(); }

is still an option...

PS: There is one fail in this updated code: The Preprocessor will not actually insert the comments at the right place.

2010-01-28 Reply Admin

but WHY, use break;

2010-01-28 Reply Admin

There are no right or wrong way. You'll have to choose your path my friend. You can be a coding GURU, hacker, or whatever path fits you.

In this case he chooses to be a computer "Wizzard", and as you may know: Wizzards use magic, a lot of it!

frits · 2010-01-28 Reply Admin

Your sarcasm > my sarcasm. I think...

2010-01-28 Reply Admin

eeth:
Whomever wrote this for loop didn't have the skills to write a ToString() in C++.

Whomever wrote this doesn't know when to write whom, and just sprinkles it pretty much anywhere.

dkf · 2010-01-28 Reply Admin

frits:
All your rules can suck it.

Oh, I'd suggest that one of them was OK. “Avoid goto unless you know what you are doing” is certainly reasonable. OTOH, I'd broaden it to “Avoid programming unless you know what you are doing” on the grounds that that's about the only truly effective strategy.

Undefined's hit rate of 1/6 is shamefully poor though.

2010-01-28 Reply Admin

eViLegion:
eeth:
Almighty save us all if dtModules ever contains 34598 rows.

not using foreach : fail ...
But, using foreach over a collection, then removing an item from that collection during the loop, is also fail.

As it is, to a lesser extent, for ‘for’, for there there are ways out

When removing an item, also use ‘--i’.
Or change the loop:

for (int i = dtModules.Rows.Count - 1; i >= 0; --i)

2010-01-28 Reply Admin

… hmm, of course, there's the “jump out of the loop” bit. And somebody else asking what if more than one item matches. Oh well, more fail :-)

2010-01-28 Reply Admin

[quote user="Undefined"]Use a FOR loop when you want the loop to execute a known number of times.

Don't fiddle with the FOR loop iterator variable.[/quote]Except when you need to.

Use a (DO) WHILE loop when you want the loop to execute an unknown number of times.

Don't put logical && operators inside a FOR loop definition.

Don't use empty exception handlers.

Avoid goto unless you know what you are doing.[/quote]The key to this is knowing when to break these rules.

cconroy · 2010-01-28 Reply Admin

Quylyn:
The value of i is used later in the function to look up a bank account to transfer funds into.
The coder's personal bank account number is in row 34598.

The comment is a diversion.

The coder is a genius.

Previous revision in source control:

            i = 34598;  //Maybe I needing later

2010-01-28 Reply Admin

Sure it's a WTF, probably RUSHED attempting to fill more than one hat DBA, NETWORK ADMIN, DEVELOPER, etc and just does not know! Also he applied for an HR position and told someone he had done some Access progamming, could be great code depending on the circumstances.

Or maybe he's a frigging idiot?

joelkatz · 2010-01-28 Reply Admin

Rene:
but WHY, use break;

At least speaking for myself, I prefer not to use 'break' because of concern that changes in the program structure may cause the 'break' to not break out far enough or to break out too far. If the control structure is simple enough that the whole thing fits on one screen easily, then fine. If not, it's kind of setting a maintenance trap.

for(...)
{
 for(...)
 {
  if(...) break;
 }
}

Consider someone maintaining this code.

If he changes the inner 'for' loop into a 'goto' loop for some reason (or the inner loop is no longer needed for other reasons), the 'break' will now break out of the outer loop. Ack! Obviously, that's not very likely if the code all fits on the screen.

Also, if he later adds another loop, the 'break' may then break out of the new loop rather than the old one. Even if the code fits on the screen and this is obvious, he may have to unnecessarily re-architect my code to make his changes.

In fact, that's probably the real reason I like the idea of named scopes and break/continue taking a named scope.

2010-01-29 Reply Admin

if you mean "while" use "while"

Cbuttius · 2010-01-29 Reply Admin

cybergandhi:
There are no right or wrong way. You'll have to choose your path my friend. You can be a coding GURU, hacker, or whatever path fits you.
In this case he chooses to be a computer "Wizzard", and as you may know: Wizzards use magic, a lot of it!

Wizzards spelt that way just see their babies jive with angel fingers whilst wishing it could be christmas everyday.

2010-01-29 Reply Admin

Undefined:
frits:
Undefined:
Use a FOR loop when you want the loop to execute a known number of times.
Don't fiddle with the FOR loop iterator variable.

Use a (DO) WHILE loop when you want the loop to execute an unknown number of times.

Don't put logical && operators inside a FOR loop definition.

Don't use empty exception handlers.

Avoid goto unless you know what you are doing.

All your rules can suck it.

This is exactly why Software Engineering is seen as a joke to the rest of the engineering world. I hope to God that you never find yourself working on something important, Mr All-Your-Rules-Can-Suck-it.

A joke where you can bill $100+ an hour without a college degree, Mr. I Have All Kinds Of Rules

2010-01-29 Reply Admin

I with the WTFs I dealt with in my job were anywhere near this benign.

2010-01-29 Reply Admin

joelkatz:
I sometimes wish C had a way to name a scope and 'break' or 'continue' a named scope.

A GOTO by any other name?

campkev · 2010-01-29 Reply Admin

Coyne:
Hans:
PS. Don't forget to stick a "#define breakto goto" somewhere ;-)

This is the best joke I've seen since that one about how to completely bamboozle someone; to wit:
#define when if

#define if while

FTFY

2010-01-29 Reply Admin

frits:
Come on. Everybody knows this situation calls for:
i = dtModules.Rows.Count + 1; //used to jump out of the for

It's way more portable and doesn't rely on magistical numbers.

This actually defeats the loop optimization, so "break" is preferable.

2010-01-29 Reply Admin

eeth:
Almighty save us all if dtModules ever contains 34598 rows.

not using foreach : fail

boxing/unboxing with ToString(): fail

comparing string versions of numbers : fail

i = 34598 instead of break isn't that horrible, performancewise - just sad.

Using foreach on a loop that does a RemoveAt on the same data structure: FAIL

2010-01-29 Reply Admin

DT:
I'd go for
try { for (int i = 0; i < dtModules.Rows.Count; i++) { if (dtModules.Rows[i]["id"].ToString() == ID.ToString()) { dtModules.Rows.RemoveAt(i); throw new BreakException(); } } } catch {}

Umm, Exceptions are REALLY inefficient. You are going to throw one every time you remove an item? This is TRWTF.

2010-01-29 Reply Admin

Slicerwizard:
TRWTF is that none of you geniuses realize that a break statement is just a goto in disguise. Thank dog the original coder was able to avoid that evil...

TRWTF is that you have NO IDEA why GOTO is evil. How old are you? 15?

GOTO is evil because you have to search the entire code for the corresponding LABEL and people do all kinds of wacky things like jumping into the middle of a FOR LOOP.

Going to the end of the current structure is hardly evil. It is simple and makes perfect sense to everyone reading.

frits · 2010-01-29 Reply Admin

PRMan:
frits:
Come on. Everybody knows this situation calls for:
i = dtModules.Rows.Count + 1; //used to jump out of the for

It's way more portable and doesn't rely on magistical numbers.

This actually defeats the loop optimization, so "break" is preferable.

Wow. You're almost smart.

2010-01-29 Reply Admin

L10: int i = 0; L20: if (i >= dtModules.Rows.Count) goto L70; L30: if (dtModules.Rows[i]["id"].ToString() == ID.ToString()) { L40: dtModules.Rows.RemoveAt(i); } L50: i = i + 1; L60: goto L20; L70: return;

<Aside>Never noticed C# doesn't allow numeric-only labels before...</Aside>

2010-01-29 Reply Admin

oh, for the love of (the non-existent) God! that's the SAME thing wow but as a joke... still fail.

joelkatz · 2010-01-29 Reply Admin

the beholder:
joelkatz:
I sometimes wish C had a way to name a scope and 'break' or 'continue' a named scope.
A GOTO by any other name?

Well, you can argue that every control structure, even functions, are a goto with another name. However, named break/continue scopes have several advantages over a goto:

They don't require the label statements, thus reducing verbosity and eliminating the risk that new code will erroneously be placed on the wrong side of the label.
They don't require you to search for the corresponding label to see what the code structure does. It's obvious (both to man and machine) from 'break' that it breaks out of a loop and from 'continue' that it continues it.
Naming the scope has other advantages, for example, the scope can be summarized by its name in a high-level code view and the close brace can be decorated with its name on hover by a clever editor.
A 'goto' label may inhibit optimizations if the compiler can't easily tell all the places it might be referenced. An extra pass through the function may be needed (or not done and optimizations left out) because the compiler assumes the label may be called from anyplace in the function scope. A named scope immediately tells the compiler that it cannot be jumped into or out of from outside that scope. (Unless a goto label appears inside it, of course.) Optimizations that are disabled by a goto label need not be disabled by naming a scope.
Named scopes are easier to learn how to use correctly than 'goto' is. Many programmers who have no clue when or how to properly use 'goto' know how to use 'break' and 'continue' just fine and could (and maybe even would) much more easily learn to use named scopes.
Many programmers who consider even clever use of 'goto' as a kind of personal failure would likely see clever use of named scopes as a kind of personal victory. (Okay, I'm joking about this one.)

But I agree that none of these factors, alone or in combination, is anything close to Earth shattering.

2010-01-30 Reply Admin

Wait, just wait a second! The loop-breaking thing is nowhere close to TRWTF. Using the loop at all it TRWTF.

This is what the code should have looked like:

DataRow drModule = dtModules.Select("id = " + ID.ToString())[0]; dtModules.Rows.Remove(drModule);

That's it. No loops, no WTFs.

2010-01-30 Reply Admin

Stev Taal:
oh, for the love of (the non-existent) God! that's the SAME thing wow but as a joke... still fail.

In a nutshell, yes.

2010-01-30 Reply Admin

Reassuringly, resharper was going nuts at me while I was proofing that joke...

2010-01-30 Reply Admin

Also, commit FAIL...

<< L40: dtModules.Rows.RemoveAt(i); }

L40: dtModules.Rows.RemoveAt(i); L45: goto L70; }

2010-01-30 Reply Admin

Loren Pechtel:
Add me to the crowd that thinks for is better than foreach here. Even if the foreach approach doesn't cause an error it's implying the wrong thing. I would never code a foreach that terminated in the middle (other than a major error bailout.) Foreach implies that you are covering the whole group!

For i = 1 to 10

Implies that you are covering the whole group, from 1 to 10.

"While" loops imply that you are going to repeat until a condition is met.

What you are advocating is a coding convention like the One True Brace Style, explicitly contradicted by the clear language design, but sanctioned by your friends and your habit.

2010-01-30 Reply Admin

PRMan:
TRWTF is that you have NO IDEA why GOTO is evil. How old are you? 15?
GOTO is evil because you have to search the entire code for the corresponding LABEL and people do all kinds of wacky things like jumping into the middle of a FOR LOOP.

Going to the end of the current structure is hardly evil. It is simple and makes perfect sense to everyone reading.

TRWTF is that have NO IDEA why GOTO is evil. How old are you? 15?

GOTO is evil because you have not idea how you got TO a particular label, or what the STATE is when you ARRIVE at a particular label. Finding a jump target in the code is trivial, and always has been.

By the way, going to the end of the current structure is explicitly what "Structured Programming" was meant to replace, and was certainly included in the idea that "GOTO considered evil". This was a contentious idea: many people at the time considered that going to the end of the current structure was hardly evil. To them, it was simple and made perfect sense.

tgape · 2010-01-31 Reply Admin

Vechni:
But why 34598? Why not 34599?

Because the developer queried all of the current users, and didn't find any who had 34599 modules. So, obviously, the number didn't need to be that high.

Either that, or the developer tried that, and it segfaulted. One less worked. ;)

2010-01-31 Reply Admin

eeth:
Addendum (2010-01-27 14:03): Thankfully most posts here are logical, otherwise I might think that trwtf is the coding community... open discussion on optimal programming technique is terrific, namecalling over grammar (or anything, really) is a bit sad.

You're new here, aren't you?

2010-02-02 Reply Admin

If the Count Property is a short than this code make prefect sence

2010-02-05 Reply Admin

ShatteredArm:
Thinking a foreach is better here than a for is a major WTF, and I dare say, even a bigger WTF than the article. There is absolutely NO reason to pick a foreach here.
Also, I thought of a more elegant solution:
bool doContinue = false;
for (int i = 0; doContinue && i < dtModules.Rows.Count; i++)
    {
        if (dtModules.Rows[i]["id"].ToString() == ID.ToString())
        {
            dtModules.Rows.RemoveAt(i);
            doContinue = false;  //used to jump out of the for
        }
    }

Erm... doContinue is false and always false. This code does nothing, now thats elegant!

2010-02-08 Reply Admin

We actually had coding guidelines banning break in such context (only to be used with switch...case ant there mandatory)... This is who those WTFs are born.

Captcha: bene... Well everthings gona be bene.

2010-02-11 Reply Admin

frits:
i = dtModules.Rows.Count + 1; //used to jump out of the for

Off by two. And no, sarcasm does not excuse an off-by-two error!!!!1!1!!1

2010-02-15 Reply Admin

Weird how it took so long. But maybe dtModules is actually short for "damn tasty modules"

2010-03-12 Reply Admin

you can modify in a foreach if you use it as a delegate

objectList.foreach(delegate(....

2010-09-16 Reply Admin

while(dtModules.Rows[i]["id"].ToString() != ID.ToString()) { i++; } dtModules.Rows.RemoveAt(i);

Break Out

Leave a comment on “Break Out”