• Anonymous (unregistered)

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

  • Bluregh (unregistered)

    The comment saves it a little.

  • SR (unregistered)

    34,598th (i.e. last)

  • (cs)

    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.

  • Addison (unregistered)

    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.

  • (cs)

    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.
  • INTERNETS (unregistered)

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

  • Steve the Cynic (unregistered)

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

    An Error Occurred An Error Occurred

  • Bluregh (unregistered) in reply to frits
    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;
        }
  • Steve the Cynic (unregistered) 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.
    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.
  • (cs) in reply to Steve the Cynic
    Steve the Cynic:
    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.
    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.

    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?

  • eViLegion (unregistered) in reply to eeth

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

  • Marc B (unregistered)

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

  • highphilosopher (unregistered) 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.

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

  • (cs) in reply to Steve the Cynic
    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!

  • (cs)

    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!

  • (cs) in reply to highphilosopher
    highphilosopher:
    <snip> WTF is wrong with a for loop? </snip>

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

  • chodger (unregistered) in reply to java.lang.Chris;
    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#.

  • Drew (unregistered)

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

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

  • Corey (unregistered)

    Or, you know:

    DELETE FROM Modules WHERE id = i
    
  • DT (unregistered)

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

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

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

  • Mike (unregistered)

    Absolutely brilliant.

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

  • Kevin (unregistered) in reply to TGV

    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.

  • ClutchDude (unregistered) 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.

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

  • Kainsin (unregistered) in reply to XIU

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

  • Phanton_13 (unregistered)

    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.

  • DT (unregistered) in reply to ClutchDude

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

  • ClutchDude (unregistered) in reply to ClutchDude
    ClutchDude:
    1)If the source is Java, foreach did not exists until 1.6

    Sorry...that should be 1.5

  • Slicerwizard (unregistered)

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

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

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

    1. That does the opposite (removes the last occurrence of ID rather than the first) which may not be an issue.
    2. i is referenced out of context after the for loop.
  • (cs) in reply to Slicerwizard
    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

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

  • Vollhorst (unregistered) in reply to Corey
    Corey:
    Or, you know:
    DELETE FROM Modules WHERE id = i</div></BLOCKQUOTE>LIMIT 1
    
  • highphilosopher (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.

    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.

  • Kempeth (unregistered)

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

  • sirlewk (unregistered) in reply to Kempeth

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

  • JAn (unregistered) in reply to Kempeth
    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!

  • (cs)

    Surely this can be handled with LINQ.

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

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

    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.

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

    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.

  • daqq (unregistered)

    Real coders use goto

Leave a comment on “Break Out”

Log In or post as a guest

Replying to comment #297124:

« Return to Article