http://blogs.msdn.com/b/brada/archive/2004/06/13/154866.aspxI enjoyed reading (and forwarding) the comments on my recent post on the productivity performance tradeoff&#8230; Many of you (rightly so) pointed out that the details really mater in such a discussion. So I picked one that we are struggling with righten-USTelligent Evolution Platform Developer Build (Build: 5.6.50428.7875)http://blogs.msdn.com/b/brada/archive/2004/06/13/154866.aspx#672212Thu, 20 Jul 2006 07:05:07 GMT91d46819-8472-40ad-a661-2c78acb4018c:672212mikechampion's weblogDon't feel bad if you don't know what the Halloween problem is.&amp;amp;nbsp; According to the Transact SQL Blog,...<img src="http://blogs.msdn.com/aggbug.aspx?PostID=672212" width="1" height="1">http://blogs.msdn.com/b/brada/archive/2004/06/13/154866.aspx#158342Thu, 17 Jun 2004 16:55:00 GMT91d46819-8472-40ad-a661-2c78acb4018c:158342Mark LevisonKeep the check in. If we've got performance problems that need work, they are likely far bigger than this little check.<div style="clear:both;"></div><img src="http://blogs.msdn.com/aggbug.aspx?PostID=158342" width="1" height="1">http://blogs.msdn.com/b/brada/archive/2004/06/13/154866.aspx#157966Thu, 17 Jun 2004 08:58:00 GMT91d46819-8472-40ad-a661-2c78acb4018c:157966Cyrus NajmabadiI agree with Poust: <br> <br>Unless you can formalize what it means to change an underlying collection while iterating over it, then you should probably throw as an unsupported operation. <br> <br>For example, if I am iterating over a tree then I expect to get every element in the tree. If someone removes an element which causes a rebalance then it's almost certainly gauranteed that the iterator will be in a hosed position at that point and that continuing the iteration will not give me all the correc telements (it might return an old element again, or it may miss elements). Because of this it's better to throw and make the consumer realize that they are almost definitely not doing the right thing. <br> <br>If you need to expose mutability semantics that work in the presense of iteration, then it needs to be something that works hand-in-hand with the iterator. For that reason I see those operations as being better suited on the iterator. If you want to add an element, tell the iterator. This will ensure that after the operation the iterator and underlying collection will be in a sane state.<div style="clear:both;"></div><img src="http://blogs.msdn.com/aggbug.aspx?PostID=157966" width="1" height="1">http://blogs.msdn.com/b/brada/archive/2004/06/13/154866.aspx#156988Wed, 16 Jun 2004 11:25:00 GMT91d46819-8472-40ad-a661-2c78acb4018c:156988W PoustLet's see... if we extend the performance argument, I should take out all the error checking in my code and all will be good and lightning fast as well. Sounds good to me; no more writing those pesky if statements. <br> <br>I just don't buy it. For our customers, stability and security go hand-in-glove. Without the error checking, you miss the obvious logic error (which is based on implementation details). Would the error be found in testing? Maybe - depends on the tests performed. Would the error be found in the field? Definitely. That's not a good place or time for discovering nuances about the underying library. <br> <br>If statements comparing values in memory for error checking are essentually free. Thank the hard working engineers at Intel for their instruction pipeline optimizations. Please sprinkle on liberally.<div style="clear:both;"></div><img src="http://blogs.msdn.com/aggbug.aspx?PostID=156988" width="1" height="1">http://blogs.msdn.com/b/brada/archive/2004/06/13/154866.aspx#155474Mon, 14 Jun 2004 22:09:00 GMT91d46819-8472-40ad-a661-2c78acb4018c:155474Frank HilemanCorrection: in the enumerator, don't check to see if Count changed -- just check to see if the enumerator is going out of bounds, by using Count.<div style="clear:both;"></div><img src="http://blogs.msdn.com/aggbug.aspx?PostID=155474" width="1" height="1">http://blogs.msdn.com/b/brada/archive/2004/06/13/154866.aspx#155473Mon, 14 Jun 2004 22:06:00 GMT91d46819-8472-40ad-a661-2c78acb4018c:155473Frank HilemanI agree essentially with Justin. We created some base collection classes in VG.net and had to make the exact same decision. We decided to remove the version number, not because it makes the loop slower, but because it makes every instance of the collection larger. This is a more important factor. That version number is not retaining useful information most of the time, and is wasteful. Why force everyone to store it? <br> <br>As Justin points out, you can eliminate any possible exceptions by making the enumerator more robust, checking to see if the Count changed. This does not prevent changes to the list, but actually adds flexibility -- limited changes might be allowed. If people really want to change the list and iterate at the same time, they can always use a copy, or for (int i = 0; i &lt;....). <br> <br>So put error checking in the enumerator, and let users use the &quot;for&quot; to get max speed. <br> <br>This argument applies to an ArrayList type of class, not a true linked list.<div style="clear:both;"></div><img src="http://blogs.msdn.com/aggbug.aspx?PostID=155473" width="1" height="1">http://blogs.msdn.com/b/brada/archive/2004/06/13/154866.aspx#155299Mon, 14 Jun 2004 18:17:00 GMT91d46819-8472-40ad-a661-2c78acb4018c:155299Philip RieckWhile I thank you for a specific case, I think this question is moot. <br> <br>We already have the choice (as pointed out above) : <br>1. Use foreach(...). I'm relying on a shortcut to produce easy code without having to take control of all the details. <br> <br>2. Use for(...). I'm taking control of the details, and I know enough about how the class (List&lt;t&gt; in this case) works to enumerate it myself. I'm doing that because I understand the issues, and need the benefits that all this extra work will get me. <br> <br> <br>It seems to me that developers in case one are clearly saying &quot;productivity for me, please&quot;, and are willing to take a small perf hit. In case #2 the developer is saying &quot;I know the risks, give me control&quot;, and can address thier own perf issues. <br> <br>I'd be more interested in an example you have where we *can't* have it both ways. I think the discussion may be a bit more heated. Personally, when choosing between perf and productivity, I want both.. :)<div style="clear:both;"></div><img src="http://blogs.msdn.com/aggbug.aspx?PostID=155299" width="1" height="1">http://blogs.msdn.com/b/brada/archive/2004/06/13/154866.aspx#155215Mon, 14 Jun 2004 16:24:00 GMT91d46819-8472-40ad-a661-2c78acb4018c:155215Keith HillKeep the check for productivity, use the for loop for performance if necessary. Also to get around the bugs with removing elements as you walk a collection, just walk it in the reverse direction.<div style="clear:both;"></div><img src="http://blogs.msdn.com/aggbug.aspx?PostID=155215" width="1" height="1">http://blogs.msdn.com/b/brada/archive/2004/06/13/154866.aspx#155140Mon, 14 Jun 2004 14:43:00 GMT91d46819-8472-40ad-a661-2c78acb4018c:155140Justin RogersTo explain why you get the NullRef instead of the IndexOutOfRangeException, you have to understand there is a larger underlying array in this example. Say it is 20 elements. As the enumerator goes through the list, it just increments an offset. Once you get to element 7, and one of your elements is already gone, it will return whatever is in the underlying array, which in this case is null. This is a very insidious issue because the enumerator uses a fixed end-index instead of a count. <br> <br>Now, you might ask, why does it happen on element 7? Well, we started with 7 elements and so endindex is giong to be 7 (6 actually). Then we removed element KCwalina, and the entire array underneath gets shifted down by 1. That puts Kit at the current offset, meaning we skip this element entirely. The next MoveNext will increment us one and put us on Peter. <br> <br>Brad actually got to demonstrate two bugs with a single sample. One bug is the array element skipping If I were using a for loop and I removed an element based on index, I would decement my index pointer, so the next time through the loop, I was placed at the same index and could process the new element. You might try to be savvy and just bump to the top of the for...loop, but this offers up the problem of whether or not this was already the last element in the list so there wouldn't be any more elements to process (a third type of issue). <br> <br>The second bug is walking past the end of the list, but still living within the bounds of the underlying array, in which case you'll get null's for object types, and even worse, 0's for integer types, and 0.0 for float types. You can see where that would really suck, because you wouldn't even know what you had done, since you won't get a NullRef and you won't get any bounding exceptions either.<div style="clear:both;"></div><img src="http://blogs.msdn.com/aggbug.aspx?PostID=155140" width="1" height="1">http://blogs.msdn.com/b/brada/archive/2004/06/13/154866.aspx#155015Mon, 14 Jun 2004 10:20:00 GMT91d46819-8472-40ad-a661-2c78acb4018c:155015David LevineFor most situations leaving the check in would provide the most benefit, especially since the 1st test run that exercised the .Remove call would throw an exception, providing instant feedback during a test of a bug. Taking out the check means that testing may not even find this if the test data does not include all the edge cases. <br> <br>Since taking out the check results in a misleading NullReferenceException this is even more reason to leave it in. When the foreach accesses the list with a bad index (beyond its end) I would have expected an IndexOutOfRangeException. Instead, it must be casting the result to a null string reference. How is the Current property implemented? Misleading information is almost as bad as no information. <br> <br>I agree with Justin - if I want high performance I'll use a for loop where I explicitly take on the responsibility of handling the list correctly. <div style="clear:both;"></div><img src="http://blogs.msdn.com/aggbug.aspx?PostID=155015" width="1" height="1">