Partilhar via


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.

Comments

  • Anonymous
    February 17, 2005
    Very intersting. It's nice to see thought went into this process. I'm glad you blogged about this, and devs will appreciate being made aware.
  • Anonymous
    February 17, 2005
    The comment has been removed
  • Anonymous
    February 18, 2005
    I like breaking changes..
    Makes me feel that something is being fixed properly instead of being worked-around.
  • Anonymous
    February 18, 2005
    I wasn't even aware of the Max Capacity value. In fact, I can't think of a situation where I would ever use it.
  • Anonymous
    February 19, 2005
    I've only used it when building a string to be P/Invoked and the particular API call limits you to a length of MAX_PATH, MAXPNAMELEN, or something similar.

    I'm not sure if this accomplishes anything, but now, if there's too much data, I get a managed exception rather than an error in the API call, which seem to vary from function to function.

    I assume there's a really good use in the framework itself for max capacity, or else it wouldn't have been included.
  • Anonymous
    January 21, 2009
    PingBack from http://www.keyongtech.com/461803-netcf-2-beta-stringbuilder-incompatibility