Runtime: Add StringBuilder append methods to StringBuilder.

[JeremyKuhne]

This is useful for more complicated string manipulation (interop in particular).

See CoreCLR issue: https://github.com/dotnet/coreclr/issues/1843
And PR: https://github.com/dotnet/coreclr/pull/2287

@terrajobst; @vancem
Per the discussion below the following is the new proposal:

(NEW) APIs being added

 namespace System.Text {
     public sealed class StringBuilder {
+       // Parameter handling to match existing string Append methods
+       public StringBuilder Append(StringBuilder value)
+       public StringBuilder Append(StringBuilder value, int startIndex, int count)
     }
 }

REJECTED Proposal:

 namespace System.Text {
     public sealed class StringBuilder {
+        public StringBuilder(StringBuilder value)
+        public StringBuilder(StringBuilder value, int capacity)
+        public StringBuilder(StringBuilder value, int startIndex, int length, int capacity) 
     }
 }

[jkotas]

It does not makes sense to add these as constructors. They should be added as Append methods instead:

• to match the existing StringBuilder surface. Append overloads are the rich set that takes all sort of arguments
• to be more flexible. Constructing one string builder from another one can be done as construct empty + append; but not the other way around.

[vancem]

When making them Appends, you don't need the capacity argument (which is nice).

Append(StringBuilder value);
Append(StringBuilder value, int startIndex, int length);

I believe we should start there. As Jan indicates this is sufficient. We may wish to have a constructor that takes a StringBuilder directly as a convenience but we should have clear reasons there is additional value there before doing that.

While you are at it, it would be very useful to have a method that writes a StringBuilder to a TextWriter (or a Stream) directly (without making a string out of it). This is a reasonably common scenario and there really pretty inefficient to materialize the strings (which is what you HAVE to do today.

Vance

[AlexRadch]

I think that creating one StringBuilder from other is not so useful, as have fast method to append one StringBuilder to other.

For example program can create many StringBuilder strings in different threads for performance and then it should append they all to one StringBuilder for result string.

Now each StringBuilder will be coped twice. One time when it will be converted to string and the second time when each string will be coped to result StringBuilder. Now stringBuilder.Append(otherStringBuilder) perform coping twice!

So I suggest to add next fast methods

 namespace System.Text {
     public sealed class StringBuilder {
+        public StringBuilder Append(StringBuilder value)
+        public StringBuilder Append(StringBuilder value, int startIndex, int count)
        //  I do not know how useful will be to add similar Insert methods also
     }
 }

If StringBuilder will have such API then creating one StringBuilder from other will be not so useful because can be replaced by next code

      var sb = new StringBuilder().Append(otherStringBuilder);
      var sb = new StringBuilder(capacity).Append(otherStringBuilder, startIndex, count);

So to add Append method is much more useful than to add constructor.

[danmosemsft]

@JeremyKuhne do you still have time to take this API proposal forward (eg change to append)? If not someone else should...

[JeremyKuhne]

@AlexRadch, @vancem, @jkotas I'm fine with just the Append. That said, there is no way to create a totally empty StringBuilder- you really need to sb2 = new StringBuilder(sb1.Length).Append(sb1). Not terribly elegant, but with anything you do you should be calculating needed capacity to get the best performance.

So lets submit these to API review:

c# namespace System.Text { public sealed class StringBuilder { // Parameter handling to match exisitng string Append methods public StringBuilder Append(StringBuilder value) public StringBuilder Append(StringBuilder value, int startIndex, int count) } }

[vancem]

I mentioned this to Jeremy offline. I am OK with just adding the Append(StringBuilder) overload for now. If we find that creating a StringBuilder from another StringBuilder is important we can add it, but I would leave it out until someone describes a plausible scenario or two for it, as you really don't want to be copying StringBuilders for the most part (and you may end up reusing it anyway).

[AlexRadch]

@JeremyKuhne you changed title of this issue, but did not change "APIs being added" section in issue description. Should be "APIs being added" section changed from creating constructors to creating Append methods also?

[JeremyKuhne]

I've updated it to make it clearer.

[AlexRadch]

I suggest next fast Append implementation https://github.com/dotnet/coreclr/pull/8140 to append one StringBuilder to other.

[jamesqo]

Nice! 👍

[terrajobst]

Looks good!

[karelz]

Warning: We approved only the Append methods, not the constructors.

[vancem]

The two new Append methods look fine

[AlexRadch]

@karelz I am working on this issue

[joperezr]

@AlexRadch are you still interested in working on this? Looks like you got some feedback on the open PR.

[karelz]

I haven's seen any reply from @AlexRadch in last 1.5 months ... unassigned, hence switching back to "up for grabs" - whoever wants to grab it, please do so.

Consider using the closed PRs above (esp. https://github.com/dotnet/coreclr/issues/8615) as a starting point for future work.

[danmosemsft]

@Anipik it makes sense for you to do this since you know a bit about string builder now..

[jkotas]

Needs to be exposed in corefx