6 ms·

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'

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 < j; i++) { this.MyControl.TabPages.Remove(this.MyControl.TabPages[i]); }

dkersten 15y ago
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.
- billybob 15y ago
  Correct. I think the logical thing to do would be to delete from the end so that the index is always valid. 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.
  - kevinkemp 15y ago
    Actually, you will never delete the final element with that code. I think what you meant was: for (int i=this.MyControl.TabPages.Count; i > 0; i--) { this.MyControl.TabPages.Remove(this.MyControl.TabPages[i-1]); }
    - dkersten 15y ago
      I prefer: for (int i=this.MyControl.TabPages.Count - 1; i >= 0; i--) { this.MyControl.TabPages.Remove(this.MyControl.TabPages[i]); } Though a simple while loop is much easier to follow, even if its less efficient than removing the elements in reverse.
      - kevinkemp 15y ago
        Although your point on efficiency stands (at least with data structures that have to reshuffle contents on deletion), the type of loop that you use has nothing to do with the order that you delete the elements. You could easily do something like: while (MyControl.TabPages.Count > 0) { MyControl.TabPages.RemoveAt(MyControl.TabPages.Count-1); } For loop are nothing more than while loops with: (1) an assignment (int i = MyControl.TabPages.Count in this case) (2) an extra command (i-- in this case) added to the end
        dkersten 15y ago
        Sure. Though if we're talking about efficiency, I would imagine that counting backwards would be slightly faster than getting the current count each time. Though in reality this depends on way too many factors - I imagine TabPages would be stored in cache and getting the count is just as fast as counting backwards. Micro-optimization and all that. Regarding for vs while, I find the choice is important only in the intent they emphasize: while puts emphasis on the condition, whereas for puts the emphasis on the iteration. I think in this case the condition (that the list is not empty) is deserves more emphasis than the iteration through the elements of said list - hence why I find the while version to be more readable. YMMV and all that :)
      - IgorPartola 15y ago
        Exactly. For example in JavaScript doing a something.length would result in re-counting the number of elements, while doing the loop in this style would efficiently store the count in a variable that is fast to access and manipulate. I suppose if you want to be a real optimization junky you'd also use --i instead of i--.
        dkersten 15y ago
        I suppose if you want to be a real optimization junky you'd also use --i instead of i--. Ten years ago, sure, but nowadays I trust the compiler to do this for me ;-)
- masklinn 15y ago
  That's also what I read of it. It's TFA's method, except broken (or not fixed, word it as you prefer).
brlewis 15y ago
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.
- simonsarris 15y ago
  For the record, the second way is typically better: It is usually more (sometimes much more) efficient to remove from the end of a list/array than to remove from the beginning.
  - thaumaturgy 15y ago
    I'm genuinely curious -- why? The last time I coded my own linked-list, it was doubly-linked, so deleting either the head or the tail was exactly the same.
    - simonsarris 15y ago
      Its heavily implementation dependent, but for many implementations of lists and arrays shift is slower than pop. Shift tends to require moving the entire array around, while pop does not. It's rarely the case with doubly-linked lists, or with perl arrays (they do something special, keep the starting offset recorded or something) For you case deleting from either end ought to be fine, but you've made the other implicit tradeoff because merely accessing items in the middle of a linked list will be slow. In the case of something like a JavaScript array, removing from the front is 80% slower than removing from the end: http://jsperf.com/popvsshift http://jsperf.com/popvsshift Same deal with Python lists. From the Python spec: http://docs.python.org/tutorial/datastructures.html#using-lists-as-queues http://docs.python.org/tutorial/datastructures.html#using-li... It is also possible to use a list as a queue, where the first element added is the first element retrieved (“first-in, first-out”); however, lists are not efficient for this purpose. While appends and pops from the end of list are fast, doing inserts or pops from the beginning of a list is slow (because all of the other elements have to be shifted by one).
      - 15y ago

dkersten 15y ago

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.

billybob 15y ago
Correct. I think the logical thing to do would be to delete from the end so that the index is always valid. 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.
- kevinkemp 15y ago
  Actually, you will never delete the final element with that code. I think what you meant was: for (int i=this.MyControl.TabPages.Count; i > 0; i--) { this.MyControl.TabPages.Remove(this.MyControl.TabPages[i-1]); }
  - dkersten 15y ago
    I prefer: for (int i=this.MyControl.TabPages.Count - 1; i >= 0; i--) { this.MyControl.TabPages.Remove(this.MyControl.TabPages[i]); } Though a simple while loop is much easier to follow, even if its less efficient than removing the elements in reverse.
    - kevinkemp 15y ago
      Although your point on efficiency stands (at least with data structures that have to reshuffle contents on deletion), the type of loop that you use has nothing to do with the order that you delete the elements. You could easily do something like: while (MyControl.TabPages.Count > 0) { MyControl.TabPages.RemoveAt(MyControl.TabPages.Count-1); } For loop are nothing more than while loops with: (1) an assignment (int i = MyControl.TabPages.Count in this case) (2) an extra command (i-- in this case) added to the end
      - dkersten 15y ago
        Sure. Though if we're talking about efficiency, I would imagine that counting backwards would be slightly faster than getting the current count each time. Though in reality this depends on way too many factors - I imagine TabPages would be stored in cache and getting the count is just as fast as counting backwards. Micro-optimization and all that. Regarding for vs while, I find the choice is important only in the intent they emphasize: while puts emphasis on the condition, whereas for puts the emphasis on the iteration. I think in this case the condition (that the list is not empty) is deserves more emphasis than the iteration through the elements of said list - hence why I find the while version to be more readable. YMMV and all that :)
    - IgorPartola 15y ago
      Exactly. For example in JavaScript doing a something.length would result in re-counting the number of elements, while doing the loop in this style would efficiently store the count in a variable that is fast to access and manipulate. I suppose if you want to be a real optimization junky you'd also use --i instead of i--.
      - dkersten 15y ago
        I suppose if you want to be a real optimization junky you'd also use --i instead of i--. Ten years ago, sure, but nowadays I trust the compiler to do this for me ;-)
masklinn 15y ago
That's also what I read of it. It's TFA's method, except broken (or not fixed, word it as you prefer).

brlewis 15y ago

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.

simonsarris 15y ago
For the record, the second way is typically better: It is usually more (sometimes much more) efficient to remove from the end of a list/array than to remove from the beginning.
- thaumaturgy 15y ago
  I'm genuinely curious -- why? The last time I coded my own linked-list, it was doubly-linked, so deleting either the head or the tail was exactly the same.
  - simonsarris 15y ago
    Its heavily implementation dependent, but for many implementations of lists and arrays shift is slower than pop. Shift tends to require moving the entire array around, while pop does not. It's rarely the case with doubly-linked lists, or with perl arrays (they do something special, keep the starting offset recorded or something) For you case deleting from either end ought to be fine, but you've made the other implicit tradeoff because merely accessing items in the middle of a linked list will be slow. In the case of something like a JavaScript array, removing from the front is 80% slower than removing from the end: http://jsperf.com/popvsshift http://jsperf.com/popvsshift Same deal with Python lists. From the Python spec: http://docs.python.org/tutorial/datastructures.html#using-lists-as-queues http://docs.python.org/tutorial/datastructures.html#using-li... It is also possible to use a list as a queue, where the first element added is the first element retrieved (“first-in, first-out”); however, lists are not efficient for this purpose. While appends and pops from the end of list are fast, doing inserts or pops from the beginning of a list is slow (because all of the other elements have to be shifted by one).
    - 15y ago