31–40 of 217 posts
Deleting from a container while you are iterating over it should always raise red flags.
Especially when you're re-calculating the count on each iteration :) Inefficient and incorrect. Double whammy. While not as efficient as while(--), the fix get's the point across: for (int i=0,j=this.MyControl.TabPages.Count; i
About the McConnell quote: "Inefficient programmers tend to experiment randomly until they find a combination that seems to work." The essence of this quote is being passed around quite often these days. When you first start programming, you generally have no idea what the hell you are doing. You learn all these strange, abstract concepts best, by experimenting. It's easy to dismiss people "jiggling things around unt…
About the McConnell quote: "Inefficient programmers tend to experiment randomly until they find a combination that seems to work." The essence of this quote is being passed around quite often these days. When you first start programming, you generally have no idea what the hell you are doing. You learn all these strange, abstract concepts best, by experimenting. It's easy to dismiss people "jiggling things around unt…
Earlier quoted context omitted.
Especially when you're re-calculating the count on each iteration :) Inefficient and incorrect. Double whammy. While not as efficient as while(--), the fix get's the point across: for (int i=0,j=this.MyControl.TabPages.Count; i
Won't this.MyControl.TabPages[i] be invalid once i becomes greater than what this.MyControl.TabPages.Count now is? Since the "always remove element 0" code from the article works, I expect your code won't work because when you remove an element form the list, all subsequent elements now have an index one less than before, so after j/2 elements, you'll overrun the list.
for (int i=this.MyControl.TabPages.Count - 1; i > 0; i--) { this.MyControl.TabPages.Remove(this.MyControl.TabPages[i]); }
Imagine the count is 2. The first iteration you delete item 1, the second you delete item 0, and then the loop exits.
EDIT: Actually, as someone else pointed out, it's clearer to use a while loop that deletes the 0th item until the collection is empty.
Earlier quoted context omitted.
Especially when you're re-calculating the count on each iteration :) Inefficient and incorrect. Double whammy. While not as efficient as while(--), the fix get's the point across: for (int i=0,j=this.MyControl.TabPages.Count; i
The problem is that .Count decrements every time you .Remove(). Either change [i] to [0] in your code, or initialize i with .Count-1 and decrement in the loop.
Earlier quoted context omitted.
Especially when you're re-calculating the count on each iteration :) Inefficient and incorrect. Double whammy. While not as efficient as while(--), the fix get's the point across: for (int i=0,j=this.MyControl.TabPages.Count; i
Won't this.MyControl.TabPages[i] be invalid once i becomes greater than what this.MyControl.TabPages.Count now is? Since the "always remove element 0" code from the article works, I expect your code won't work because when you remove an element form the list, all subsequent elements now have an index one less than before, so after j/2 elements, you'll overrun the list.
It's TFA's method, except broken (or not fixed, word it as you prefer).
Anyone else read it and on the first pass think, "yeah that works". Then on second pass think, "it's not a good idea, but it works." Then finally think, "under pressure I've done worse; at least this works as intended. S/he should probably comment it." Or is it only me? Addendum: I would also add that even as a junior program, Clear() was easily learned within the first few minutes and usually when you have to use a…
No, I pretty much just recoiled in horror immediately. Even if I had to avoid .Clear(), there are immediately-obvious, better ways. while ( !thing.Empty() ) thing.Remove( 0 ); In fact, the code isn't just bad; it's risky. What if someone changes "int" to "uint"? It wouldn't even infinite loop / crash... It would remove exactly one element!
Like the blog author, I thought it was obvious what the problem was: Every time I read that, I'm going to have to figure out what it means. Any time there's a problem or change to code in that area, I have to stop and understand what it's doing. To clean it up, I'd do 1 of 2 things: Either write a .clear() function, or rewrite it to start at the end and clear the items in reverse. With the .clear() function, I can at…
The other nice thing about running it in reverse is that the .count method (assuming it's a method and not a property), needs only to be accessed in the initial condition setup. If you're accessing the size of, say, a linked list, you end up sneaking an O(n^2) runtime because it has to re-count the size of the list every iteration to check for termination. c.f. https://secure.wikimedia.org/wikipedia/en/wiki/Schlemiel…
You're still going to run in O(n^2) if you remove from the back (as you'd do if you run in reverse).
Linked list are far more efficient if you build in reverse but remove in iteration order (just replace your head pointer with head.next)
What's wrong with the code, then, is that it was written under a little too much pressure for the developer to think clearly. for ( int i=0 ; i
Why not just: while (this.MyControl.TabPages.Count > 0) { this.MyControl.TabPages.Remove ( 0 ); }