• (cs) in reply to eeth

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

  • (cs) in reply to Corey
    Corey:
    Or, you know:
    DELETE FROM Modules WHERE id = i</div></BLOCKQUOTE>
    

    Yes, this would be my first choice, if the language allowed it.......

  • Reality Fault (unregistered) in reply to Anonymous
    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.
  • Rose This (unregistered) in reply to NeoMojo
    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.

  • Spivonious (unregistered)

    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.

  • Montoya (unregistered) in reply to illtiz
    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.

  • Anon (unregistered)

    34,598 modules should be enough for anybody.

  • Noted (unregistered) in reply to blar blar
    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.

  • Anonymous (unregistered) in reply to Plz Send Me The Code
    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();
    
  • Jack (unregistered)

    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!

  • (cs)

    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.

  • (cs)

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

  • (cs)

    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.

  • Duke of New York (unregistered)

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

  • Sutherlands (unregistered) in reply to eeth
    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.
  • Wiseguy (unregistered) in reply to eeth

    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

  • (cs) in reply to Sutherlands
    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.

  • sino (unregistered) in reply to eeth
    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.

  • (cs) in reply to sino
    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.

  • mh (unregistered) in reply to smxlong
    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.

  • hadtodoit (unregistered)

    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!

  • (cs) in reply to Anonymous

    But why 34598? Why not 34599?

  • Hans (unregistered) in reply to joelkatz
    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 ;-)

  • (cs)

    "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!

  • Anonymous (unregistered)

    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
    
  • Infinite Wisdom (unregistered) in reply to Anonymous
    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.

  • Anonymous (unregistered) in reply to Infinite Wisdom

    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 }

  • Swedish tard (unregistered) in reply to Anonymous
    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. :)

  • PS (unregistered)

    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.

  • chunder thunder (unregistered) in reply to smxlong
    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.

  • (cs) in reply to Marc B
    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.

  • sino (unregistered) in reply to eeth
    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?

  • tracker1 (unregistered) in reply to eeth

    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.

  • (cs) in reply to joelkatz
    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?

  • (cs) in reply to daqq
    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.

  • Anonymous coward (unregistered)

    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!

  • Anonymous coward (unregistered)

    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
    { 
    }
  • (cs) in reply to Hans
    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
  • Quylyn (unregistered)

    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.

  • win (unregistered) in reply to DT
    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.

  • Gaspar (unregistered) in reply to eeth

    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.

    =)

  • Aussie Contractor (unregistered) in reply to Marc B
    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)

  • (cs) in reply to eeth
    eeth:
    Almighty save us all if dtModules ever contains 34598 rows.
    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.

    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.

  • (cs) in reply to eViLegion

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

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

  • Undefined (unregistered)

    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.

  • Anon (unregistered) in reply to wonkoTheSane
    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..

  • (cs) in reply to Undefined
    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.

  • Undefined (unregistered) in reply to frits
    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.

  • (cs) in reply to Thg
    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#...

  • David Cameron (unregistered)

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

    /sarcasm

Leave a comment on “Break Out”

Log In or post as a guest

Replying to comment #:

« Return to Article