A Breaking Change we did in StringBuilder [Kit George]

Once again, we continue to have robust, passionate discussions around the breaking change bar for fixes. The BCL team itself recently had a heated discussion between the Program Manager, Tester, and Developer, surrounding a particular change that I thought I'd share: it's just an interesting experience that I think can help clarify, we really do go to bat for you, our customers. It's simply a matter of trying to decide what we think is the right thing to do.

We had a pretty silly bug in V1.0 and V1.1, for the Stringbuilder constructor that takes a maximum capacity:

// the first int is the initial capacity, the second is the max
StringBuilder sb = new StringBuilder(0,4);   

Throwing aside the question of the usefulness of having such a small builder, there's a problem with how we did this in V1.0 and V1.1. Basically, let's imagine this code:

StringBuilder sb = new StringBuilder(0,4);
Console.WriteLine(sb.Capacity); //Will print 16 on V1.1, will print 4 on V2.0
sb.Append("Hello Everyone!"); //ArgumentOutOfRangeException on V2.0
sb.Append("How are you today?"); //ArgumentOutOfRangeException on V1.1

Notice that the change we've made results in the exception occurring in a different location. The reason is brought to bear by a minor change we made to the maximum capacity effect on the initial capacity of our builder. In V1.1, we always set the default capacity on the stringbuilder if you specified the initial capacity should be zero, which is why V1.1 reports the capacity is 16. But clearly, this is useless if the maxcapacity has also been set to something which is LESS than that. The bug in V1.1 arises in that basically, we did the check for maxcapacity AFTER we check whether there's space in the builder to append more data. This is why we allow the first line to be written in V1.1, the append is less than 16 characters. At the time we need to expand the capacity automatically, we do the maxcapacity check, which is why it used to fail on the second line in V1.1

We ended up deciding the change is a good one. While on the compatibility side we have a new exception that can occur in different locations (something we always try to avoid), on the flipside were the arguments that a) we were not matching the ECMA standard and with this change we're inline with the standard, and b) the scenario is unbelievably cornercase. The capacity value must be zero, and the maxcapacity must be less than 16 (in all other situations, there is no change in behavior). The latter won out.