The Daily WTF: Curious Perversions in Information Technology

Kiddibeik · 2010-01-27 Reply Admin

Regarding your first point.

Last time I checked you couldn't use a foreach loop if you modify the collection within the loop's body (like say by removing an object from it).

DWalker59 · 2010-01-27 Reply Admin

Corey:
Or, you know:
DELETE FROM Modules WHERE id = i</div></BLOCKQUOTE>
Yes, this would be my first choice, if the language allowed it.......

2010-01-27 Reply Admin

Anonymous:
foreach (string str in myList)
{
  Console.WriteLine("Item is " + str);
  myList.Remove(str);
}
...the exception will be thrown on the call to List.Remove().

Well, yeah! There is no List. Only myList.

2010-01-27 Reply Admin

NeoMojo:
Anonymous:
NeoMojo:
How would you react if the code was like this? <snipped>
I would fire you.

You can't fire me, as I'm your bosses son, and your boss has rose tinted spectacles when it comes to me.

What do you do now?

Replace my boss. But TRWTF is waiting until something like this to do so.

2010-01-27 Reply Admin

This code won't work. It needs to go through the rows collection backwards. Removing item 0 shifts the indexes of all of the other items.

2010-01-27 Reply Admin

illtiz:
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 :-)
Well clearly the solution is to create a database, create a table, insert all of this data into that table, use SQL to delete the row that has the id, then select out what is left in the table, and drop the database. SQL is the right tool for the job, you just have to use it.

2010-01-27 Reply Admin

34,598 modules should be enough for anybody.

2010-01-27 Reply Admin

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

Both FOR and FOREACH loops are sequential scans. The real issue is why the code didn't use a more efficient look-up method for the "modules" data set.

This WTF only really matters when there is a large "modules" set to process. In that case, the high-number stop can collide with a real cell. Otherwise, this code sadly satisfies the BREAK requirement.

2010-01-27 Reply Admin

Plz Send Me The Code:
I'd have done it all in sql. That's what it is there for.

Oh, yeah, like this (pseudo code) - this avoids any potential concurrent modification exception and pushes the logic to the db, so any bottlenecks aren't our fault.

// Make sure we lock the table to make sure no one else is using it
sql.lock("tmp_table");
foreach (row in rows) {
    // Assume tmp_table exists and has an id and varchar column
    // parameterize to prevent sql injection
    sql.statement("insert into tmp_table values (%1, %2)", rows.id, rows.toString).invoke();
    sql.commit();
}
sql.statement("delete from tmp_table where id = %1", ID).invoke();
sql.commit();
rows.clear();
int[] sqlRows = sql.statement("select id, value from tmp_table").invoke();
foreach (row in sqlRows) {
    rows.add(parseStringAsRow(row));
}

//Note to future maintainers! DON'T FORGET TO EMPTY TABLE!!!!
sql.statement("delete from tmp_table");
sql.commit();

2010-01-27 Reply Admin

The fact that this is even allowed is weird: normally a compiler should check for this, and not allow other statements to change the loop control variable!

smxlong · 2010-01-27 Reply Admin

I agree that using a magic number to do this is weird, but there is really a big difference between "I want to exit this loop NOW" and "I want this to be the last iteration through this loop."

In this example, there was nothing else to be done, but in another situation there may have been other processing within the loop that still needs to happen, even though we want this to be the last pass through the loop.

Using this pattern (in a less moronic way) is not a WTF in and of itself.

ObiWayneKenobi · 2010-01-27 Reply Admin

The code's C#: dt is the Hungarian notation prefix for a DataTable.

Zolcos · 2010-01-27 Reply Admin

There's no reason to iterate in the first place. Surely there is a function (either in the library or part of the collection's class) that can find the key of the first element of a collection that causes a given predicate to evaluate to true.

2010-01-27 Reply Admin

A truly innovative programmer would have used FILENAME_MAX instead of a random constant.

2010-01-27 Reply Admin

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

Whoever wrote this sentence didn't have the skills to write a sentence using whomever.

2010-01-27 Reply Admin

Well, actually you can't modify a collection if you use it with foreach, so the for-loop is completely valid.

delenit: a hermit with delete button

eeth · 2010-01-27 Reply Admin

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

You're absolutely correct. Good thing I was a CS + music theory double major, not English.

Also, wow @ teh hostility.

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.

Nope, you're dumb.

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

This.

eeth:
chodger:
[...snip...]
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.

It's "Whoever wrote this [...]". Sometimes objects are subjects. There's a case for that.

tl;dr: Stoopid troll is stoopid.

eeth · 2010-01-27 Reply Admin

sino:
Nope, you're dumb.
tl;dr: Stoopid troll is stoopid.

Beautiful. Notice that in all of my posts I never resorted to namecalling.

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.

2010-01-27 Reply Admin

smxlong:
I agree that using a magic number to do this is weird, but there is really a big difference between "I want to exit this loop NOW" and "I want this to be the last iteration through this loop."
In this example, there was nothing else to be done, but in another situation there may have been other processing within the loop that still needs to happen, even though we want this to be the last pass through the loop.

Using this pattern (in a less moronic way) is not a WTF in and of itself.

I had a bet with myself on how many posts it would be before someone tried defending it. I won. :D

This is so obviously a clear case of a manager deciding that break was a form of goto, and therefore was forbidden.

2010-01-27 Reply Admin

I see it do you!

34,598

well 4+5 is 9 and another 9 is 18 and then if you add 8 its 26.. then if i subtract 3..

23!

Vechni · 2010-01-27 Reply Admin

But why 34598? Why not 34599?

2010-01-27 Reply Admin

joelkatz:
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.)

Now consider a modification where the loop index is used after the deletion. Maybe to generate a message to the user: "Row {rownumber} was deleted." It will always tell you that row 34598 was deleted!

Basically you cannot defend your code from random future changes.

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

It does. You can put a label where you want the scope to end, and then you use "breakto label;" when you want to break out of the scope. Try it!

Hans

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

lolwtf · 2010-01-27 Reply Admin

"wow, that’s more wrong than I could have ever imagined" It's not a goto. It's not continuing to iterate through the remaining items after it finishes. It's not self-modifying code. You clearly don't have much of an imagination!

2010-01-27 Reply Admin

For a more reliable version replace the assignment:

  i += 34598;  //used to jump out of the for

or perhaps even:

  i *= 34598;  //used to jump out of the for

2010-01-27 Reply Admin

Anonymous:
For a more reliable version replace the assignment:
  i += 34598;  //used to jump out of the for
or perhaps even:
  i *= 34598;  //used to jump out of the for</div></BLOCKQUOTE>
Couldn't you
  i = 1/0;  //REALLY big number
That should work pretty much forever.

2010-01-27 Reply Admin

This is even better:

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

2010-01-27 Reply Admin

Anonymous:
This is even better:
try { for (int i = 0; i < dtModules.Rows.Count; i++) { if (dtModules.Rows[i]["id"].ToString() == ID.ToString()) { dtModules.Rows.RemoveAt(i); throw 1/0; } } } catch(...) { //used to jump out of the for }

I still like mine completely without for loop best. :)

2010-01-27 Reply Admin

What boggles my mind is that he clearly knew how to get the actual number of rows, like he did in the first line. So what line of thought led him to that monstrosity, instead of doing at least

i = dtModules.Rows.Count;

Captcha: consequat ... this practically begs for some kind of witty pun, but I can't think of one. Oh bummer.

2010-01-27 Reply Admin

smxlong:
I agree that using a magic number to do this is weird, but there is really a big difference between "I want to exit this loop NOW" and "I want this to be the last iteration through this loop."
In this example, there was nothing else to be done, but in another situation there may have been other processing within the loop that still needs to happen, even though we want this to be the last pass through the loop.

Using this pattern (in a less moronic way) is not a WTF in and of itself.

IMO the better way to do it is to use a state variable. Not sure of the C# syntax (if that is what this is), but if it follows C:

done=0;
for(i=0; i<max && !done; i++) {
  if(stuff) { done=1; }
}</pre>
This way no magic numbers, no gotos, and the break is guaranteed to hit the right level without interfering with other flow.

bob171123 · 2010-01-27 Reply Admin

Marc B:
#define BREAK 34598
for (int i = 0; i < dtModules.Rows.Count; i++) { if (dtModules.Rows[i]["id"].ToString() == ID.ToString()) { dtModules.Rows.RemoveAt(i); i = BREAK; //used to jump out of the for } }

There, fixed.

FTFY. Actually, it does look sort of offensive. No wonder he didn't opt for break.

2010-01-27 Reply Admin

eeth:
sino:
Nope, you're dumb.
tl;dr: Stoopid troll is stoopid.

Beautiful. Notice that in all of my posts I never resorted to namecalling.

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.

Yeah, good on yah, buddy! No name-calling for you, just troll baiting with idiotic absolutes and incorrect assertions.

Also, wow @ teh little emo sensie that found his way to the TDWTF.

You've really got to get that sand out of your vagina; it's making you cranky. Does it itch?

2010-01-27 Reply Admin

But removing the item during a foreach iteration is usually bad form... the for loop is fine, as a foreach won't give you the index of the item to remove. However break; or i=Integer.MaxValue or something similar is better than an arbitrary magic number.

Heron · 2010-01-27 Reply Admin

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

Consider:

for(int i = 0; i < max; ++i)
{
   for(int j = 0; j < inner_max; ++j)
   {
      if(some_condition(i, j))
      {
         goto outerbreak;
      }
      else if(another_condition(i, j))
      {
         goto outercontinue;
      }

      do_something();
   }

outercontinue:
}
outerbreak:

How would "named scopes" be less verbose?

Coyne · 2010-01-27 Reply Admin

daqq:
Real coders use goto

Real programmers program on the bare metal with jumps and self-modifying code. ReadThe Story of Mel - you'll get the idea.

2010-01-27 Reply Admin

Should just do it like this:

bool found=false;
for (int i = 0; i < 34598; i++)
    {
        if (!found && i<dtModules.Rows.Count && dtModules.Rows[i]["id"].ToString() == ID.ToString())
        {
            dtModules.Rows.RemoveAt(i);
            found = true;
        }
    }</pre>
Advantage is that it runs in constant time!

2010-01-27 Reply Admin

Or better like this - avoids the magic number and works with large vectors:

try {
    for (int i = 0; i < dtModules.Rows.Count; i++)
    {
        if (dtModules.Rows[i]["id"].ToString() == ID.ToString())
        {
            dtModules.Rows.RemoveAt(i);
            i = -2;  //used to jump out of the for
        }
    }
catch (...) // error caused when dtModules.Rows[-1] is accessed
{ 
}

Coyne · 2010-01-27 Reply Admin

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

2010-01-27 Reply Admin

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.

2010-01-27 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 {}

Congratulations, sir, you win this thread.

2010-01-27 Reply Admin

Sure they did, you can write anything and call the method toString(). In fact, I would love to see the toString() method they wrote, I am sure it is a hoot.

=)

2010-01-27 Reply Admin

Marc B:
#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 } }

for (int i = 0; i < dtModules.Rows.Count; i++) { if (dtModules.Rows[i]["id"].ToString() == ID.ToString()) { dtModules.Rows.RemoveAt(i); goto i34598;
} }

:i34598 //used to jump out of the for

even better (although I did read somewhere that it may be something something)

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

Im sure some one else has pointed this out by now.. but.. foreach would throw and exception if you removed an item from the collection that was being looped through.

wonkoTheSane · 2010-01-27 Reply Admin

[quote user="eViLegion"][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. [/quote]

Damnit..

2010-01-27 Reply Admin

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.

2010-01-27 Reply Admin

wonkoTheSane:
Im sure some one else has pointed this out by now.. but.. foreach would throw and exception if you removed an item from the collection that was being looped through.

Yes. And also pointed out (correctly) that it doesn't throw an exception in this case, because the loop exits before the iterator is called again.

Double Damnit..

frits · 2010-01-27 Reply Admin

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.

2010-01-27 Reply Admin

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.

RogerWilco · 2010-01-27 Reply Admin

Thg:
Maybe the developer was coding for an embedded environment ... one for which the full set of grammar had not been implemented in its compiler?

Then it could have been written for a RapidHare processor circa 2003.

Oh sorry, no I'm mistaken, that was a "fully fledged" C compiler that didn't even have malloc, while this is C++/Java/C#...

2010-01-27 Reply Admin

Yeah, I see the mistake. He should have use int.MaxValue.

/sarcasm

Break Out

Leave a comment on “Break Out”