The Daily WTF: Curious Perversions in Information Technology

2010-01-27 Reply Admin

I would argue that the FOR loop is pretty much the first thing you should learn in any language. This coder clearly disagrees.

2010-01-27 Reply Admin

The comment saves it a little.

2010-01-27 Reply Admin

34,598th (i.e. last)

frits · 2010-01-27 Reply Admin

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.

2010-01-27 Reply Admin

Maybe he was confused as to which number would be just above what would fit in a 2 byte storage (32,768). Of course, that only deals with the seemingly randomness of the number and not the insane ignorance of basic concepts.

eeth · 2010-01-27 Reply Admin

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.

2010-01-27 Reply Admin

Bravo, that's the first WTF that's actually made me laugh out loud in a while.

2010-01-27 Reply Admin

What the heck, at least he wrote a comment that explained why he was doing it.

An Error Occurred An Error Occurred

2010-01-27 Reply Admin

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

Why use addition when for loops can be used to solve anything.

    for (int j = 0; j < dtModules.Rows.Count; j++)
    {
        i++;
        if (i >= dtModules.Rows.Count)
            j = 34598;
    }

2010-01-27 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.

Looks feasibly like it could be C++, which does not have foreach.
Might be incompatible types. As an excuse, this is weak, I know, especially in C++ where we could write an operator ==() that compares the two types.
They are IDs, not numbers. Why assume that IDs are numbers?
It is worse than sad. It will one day cause a weirdrandomfailure.

java.lang.Chris; · 2010-01-27 Reply Admin

Steve the Cynic:
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.

Looks feasibly like it could be C++, which does not have foreach.

Might be incompatible types. As an excuse, this is weak, I know, especially in C++ where we could write an operator ==() that compares the two types.

They are IDs, not numbers. Why assume that IDs are numbers?

It is worse than sad. It will one day cause a weirdrandomfailure.

If it was C++, the removal method wouldn't be as succinct. The preferred idiom would be some crap like:

dtModules.Rows.back_remover(dtModules.Rows.end_iter(), dtModules.Rows.end_iter() - 1);

And this code would of course be hidden in a functor class that requires half a dozen templated parameters to initialise it.

I fscking hate C++. In fact I hate it so much I've started twitching uncontrollably. Can someone pull the screens round me and find the nurse?

2010-01-27 Reply Admin

[quote user="eeth"]Almighty save us all if dtModules ever contains 34598 rows.

not using foreach : fail ...

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

2010-01-27 Reply Admin

#define END_FOR_LOOP 34598

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

There, fixed.

2010-01-27 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.

WTF is wrong with a for loop?

captcha: nulla -- Valley girl for Null (i.e. "That variable was so nulla, it like gave me an Object reference not set to an object error.").

eeth · 2010-01-27 Reply Admin

1. Looks feasibly like it could be C++, which does not have foreach. 2. Might be incompatible types. As an excuse, this is weak, I know, especially in C++ where we could write an operator ==() that compares the two types. 3. They are IDs, not numbers. Why assume that IDs are numbers? 4. It is worse than sad. It will one day cause a weirdrandomfailure.

1: If my memory serves, C++ has no ToString() 2: Conceded. 3: If they're not numbers then they're already string and don't need the ToString(). 4: Hence the initial invocation of the Almighty!

Quicksilver · 2010-01-27 Reply Admin

probably he first tried: i = Integer.MAX_VALUE;

but found out that this did not work as expected. Naturally i = 34598 was the next choice!

eeth · 2010-01-27 Reply Admin

highphilosopher:
<snip> WTF is wrong with a for loop? </snip>

Nothing's wrong with a for loop, but a foreach is generally more efficient.

2010-01-27 Reply Admin

java.lang.Chris;:
If it was C++, the removal method wouldn't be as succinct. The preferred idiom would be some crap like:
dtModules.Rows.back_remover(dtModules.Rows.end_iter(), dtModules.Rows.end_iter() - 1);

And this code would of course be hidden in a functor class that requires half a dozen templated parameters to initialise it.

I fscking hate C++. In fact I hate it so much I've started twitching uncontrollably. Can someone pull the screens round me and find the nurse?

Whilst I share your hate of template rubbish like that, which seems to have infected a lot of the standard ideas for C++, I do feel like pointing out that it's not the fault of the language. It's the fault of the libraries, including the standard library.

Qt's list container has a lovely removal function - http://doc.trolltech.com/4.6/qlist.html#removeAt (I can't link it without being marked as spam). It's short, obvious and doesn't vomit template code all over your face and torso.

Anyway, the article code is probably C#.

2010-01-27 Reply Admin

What happens if there are two objects with the corresponding ID? D:

TGV · 2010-01-27 Reply Admin

eeth:
1) not using foreach : fail 2) boxing/unboxing with ToString(): fail 3) comparing string versions of numbers : fail 4) i = 34598 instead of break isn't that horrible, performancewise - just sad.

I just cannot agree with item 1. I've go no idea what language this is (C#?), but foreach does precisely the same. Only this code also makes it clear that it is the first occurrence of ID that will be deleted. With foreach, it stays more implicit IMO. And breaking out of a foreach sort of contradicts the term itself...

2010-01-27 Reply Admin

eeth:
1: If my memory serves, C++ has no ToString()

You can make a ToString function in C++.

eeth:
3: If they're not numbers then they're already string and don't need the ToString().

Top computer scientists across the world have discussed the possibility that there may be other data types than just ints and strings.

2010-01-27 Reply Admin

Or, you know:

DELETE FROM Modules WHERE id = i

2010-01-27 Reply Admin

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 {}

XIU · 2010-01-27 Reply Admin

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

not using foreach : fail ...

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

That only matters if you continue in the foreach, so removing and then break works.

2010-01-27 Reply Admin

Absolutely brilliant.

eeth · 2010-01-27 Reply Admin

chodger:
You can make a ToString function in C++.
Top computer scientists across the world have discussed the possibility that there may be other data types than just ints and strings.

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

There are absolutely more data types than integers and strings. That's not what I said, though. And fyi, even strings reduce to numeric representations.

2010-01-27 Reply Admin

I disagree with number 1 for a different reason. If you use a foreach, you generate an enumerator. While iterating on the enumerator you cannot modify the original collection. At least you can't in C#, it generates a runtime exception.

TRWTF is the i=34598, the string comparison may be fail, but we don't actually know what the data type of the second dimension of the dictionary is, so it could actually be a reference type, in which case, the code maybe correct in doing a ToString() in order to compare it to a number.

2010-01-27 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.

1)If the source is Java, foreach did not exists until 1.6 2)Even more puzzling....is it really checking to see if the String references are the same? Again, assuming Java, shouldn't it be

if (dtModules.Rows[i]["id"].ToString().equals(ID.ToString()))

if Strings are really required for the comparison... 3)Which makes point 2 even weirder...why bother with toString instead just doing the quick arithmetic comparison? 4)I feel better about my day now...but not by much.

3rd attempt...

2010-01-27 Reply Admin

The programmer needs the index of the item in order to remove it, which is not possible with foreach.

2010-01-27 Reply Admin

for (int i = dtModules.Rows.Count-1; i && dtModules.Rows[i]["id"].ToString() != ID.ToString() ; i--); if (dtModules.Rows[i]["id"].ToString() == ID.ToString()) dtModules.Rows.RemoveAt(i);

I know, is a little weird.

2010-01-27 Reply Admin

In C#, all strings with the same "contents" refer to the same object. C# strings have an .Equals() method too, by the way.

2010-01-27 Reply Admin

ClutchDude:
1)If the source is Java, foreach did not exists until 1.6

Sorry...that should be 1.5

2010-01-27 Reply Admin

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...

2010-01-27 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...

So is a for-loop

2010-01-27 Reply Admin

Phanton_13:
for (int i = dtModules.Rows.Count-1; i && dtModules.Rows[i]["id"].ToString() != ID.ToString() ; i--); if (dtModules.Rows[i]["id"].ToString() == ID.ToString()) dtModules.Rows.RemoveAt(i);
I know, is a little weird.

That does the opposite (removes the last occurrence of ID rather than the first) which may not be an issue.
i is referenced out of context after the for loop.

eeth · 2010-01-27 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...

Pfff. ON ERROR RESUME NEXT

2010-01-27 Reply Admin

Corey:
Or, you know:
DELETE FROM Modules WHERE id = i</div></BLOCKQUOTE>
Generally, yes. Only that the code did not appear to actually change the database. It only discards a row locally. For all we know, there is no database, only some top notch coder feeling all important by using Row as a member name :-)

2010-01-27 Reply Admin

Corey:

Or, you know:

DELETE FROM Modules WHERE id = i</div></BLOCKQUOTE>LIMIT 1

2010-01-27 Reply Admin

eeth:
highphilosopher:
<snip> WTF is wrong with a for loop? </snip>

Nothing's wrong with a for loop, but a foreach is generally more efficient.

Easier to read, but if you'll check it out you'll find that a foreach usually takes longer to execute than a for loop. Although picking one over the other is such a micro-optimization that's its laughable.

2010-01-27 Reply Admin

Amateurs! OBVIOUSLY you do it best by:

Serializing the module list into XML.
Splitting the XML into a String array of lines
Searching the lines with a for-case construct until a Regex finds the id you're looking for
Then Recombining all lines except the one you found to have the right ID using another for loop.
Deserializing the result back into the object
And finally throwing a new ExitLoopException which you swallow with a empty catch-all.

2010-01-27 Reply Admin

eeth:
1. Looks feasibly like it could be C++, which does not have foreach. 2. Might be incompatible types. As an excuse, this is weak, I know, especially in C++ where we could write an operator ==() that compares the two types. 3. They are IDs, not numbers. Why assume that IDs are numbers? 4. It is worse than sad. It will one day cause a weirdrandomfailure.

1: If my memory serves, C++ has no ToString() 2: Conceded. 3: If they're not numbers then they're already string and don't need the ToString(). 4: Hence the initial invocation of the Almighty!

1: It does after you implement it. 3: It could be an ID class that stores the ID differently.

2010-01-27 Reply Admin

This was probably on an embedded system, where the programmer couldn't rely on having a filesystem.

2010-01-27 Reply Admin

Kempeth:
Amateurs! OBVIOUSLY you do it best by: * Serializing the module list into XML. * Splitting the XML into a String array of lines * Searching the lines with a for-case construct until a Regex finds the id you're looking for * Then Recombining all lines except the one you found to have the right ID using another for loop. * Deserializing the result back into the object * And finally throwing a new ExitLoopException which you swallow with a empty catch-all.

You are so hired!

frits · 2010-01-27 Reply Admin

Surely this can be handled with LINQ.

2010-01-27 Reply Admin

highphilosopher:
eeth:
highphilosopher:
<snip> WTF is wrong with a for loop? </snip>

Nothing's wrong with a for loop, but a foreach is generally more efficient.

Easier to read, but if you'll check it out you'll find that a foreach usually takes longer to execute than a for loop. Although picking one over the other is such a micro-optimization that's its laughable.

It depends on the language and how they're implemented. Generally speaking, I use a foreach when dealing with hashes and fors when dealing with arrays.

I think PHP's manual used to recommend using for loops since foreach was a little bit slower for large arrays. I can't find proof of this, but that's what memory tells me.

2010-01-27 Reply Admin

eeth:
1) not using foreach : fail

You can't use a foreach statement over the Rows collection because the collection gets changed in the loop (item removed). You will get a compilation error if you attempt to add or remove an item from a collection that you are iterating over with a foreach statement. Using a for loop is not incorrect here, it is the only sensible option.

joelkatz · 2010-01-27 Reply Admin

I somewhat sympathize with the choice to assign the loop index a "too high" value rather than using 'break'. The 'break' statement can create a trap for a maintainer. If he later adds a loop, the 'break' will break out of his inner loop rather than the outer loop, as intended.

Consider, hypothetically, if each row is later changed to contain an array of values, a match on any one of which would select that row. Someone quickly modifying all instances of accesses to the row number to iterate through all values in each row might create a subtle bug where the break doesn't prevent processing of subsequent rows. (Instead it would only break out of the new inner loop and stop checking other numbers for the current row.)

That said, alternative solutions aren't so pretty either. In some cases, 'goto' really is best, but obviously that is pretty ugly. Wrapping the loop into a function that can 'return' is sometimes suitable, but also sometimes completely inappropriate.

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

2010-01-27 Reply Admin

Anonymous:
eeth:
1) not using foreach : fail
You can't use a foreach statement over the Rows collection because the collection gets changed in the loop (item removed). You will get a compilation error if you attempt to add or remove an item from a collection that you are iterating over with a foreach statement. Using a for loop is not incorrect here, it is the only sensible option.

Sorry, it's not a compilation error but a runtime error. Either way, it won't work.

2010-01-27 Reply Admin

How would you react if the code was like this?

int i = 0;

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

dtModules.Rows.RemoveAt(i);

No for or foreach! Crazy.

If this was recent c# you wouldn't need a for or foreach (with linq, etc.), but I guess the dev might not have realised that.

2010-01-27 Reply Admin

Real coders use goto

Break Out

Leave a comment on “Break Out”