StringBuilder icon indicating copy to clipboard operation
StringBuilder copied to clipboard

Remove `ValueStringBuilder.Concat`?

Open Joy-less opened this issue 11 months ago • 5 comments

Just wondering if this could be removed, since string.Concat seems to do pretty much the same thing, except for the fact that it might box value types.

Joy-less avatar Feb 20 '25 20:02 Joy-less

Just wondering if this could be removed, since string.Concat seems to do pretty much the same thing, except for the fact that it might box value types.

Yes exactly - and that is the only reason those functions exist. I wanted to create a small set of helper methods where people can easil just append a few things without mich overhead.

Maybe no one uses them :) that might be true. Then we could remove them in a new major version

linkdotnet avatar Feb 20 '25 21:02 linkdotnet

If they are being kept, you should replace Concat<T>(params T[]) with Concat<T>(params ReadOnlySpan<T>) to avoid allocations :)

Joy-less avatar Feb 20 '25 23:02 Joy-less

If they are being kept, you should replace Concat<T>(params T[]) with Concat<T>(params ReadOnlySpan<T>) to avoid allocations :)

Good point - we could also use params Span<T>. The upcoming C# version may have some special handling where arrays would be automatically cast to Span if appropriate. I will flag this as v3

linkdotnet avatar Feb 21 '25 07:02 linkdotnet

If they are being kept, you should replace Concat<T>(params T[]) with Concat<T>(params ReadOnlySpan<T>) to avoid allocations :)

Good point - we could also use params Span<T>. The upcoming C# version may have some special handling where arrays would be automatically cast to Span if appropriate. I will flag this as v3

Hi @linkdotnet, arrays are already implicitly cast to ReadOnlySpan and Span.

char[] array = new char[4];
ReadOnlySpan<char> span = array;

I'm not sure what you're referring to by:

The upcoming C# version may have some special handling where arrays would be automatically cast to Span if appropriate.

Joy-less avatar Feb 21 '25 23:02 Joy-less

If they are being kept, you should replace Concat<T>(params T[]) with Concat<T>(params ReadOnlySpan<T>) to avoid allocations :)

Good point - we could also use params Span<T>. The upcoming C# version may have some special handling where arrays would be automatically cast to Span if appropriate. I will flag this as v3

Hi @linkdotnet, arrays are already implicitly cast to ReadOnlySpan and Span.

char[] array = new char[4]; ReadOnlySpan span = array; I'm not sure what you're referring to by:

The upcoming C# version may have some special handling where arrays would be automatically cast to Span if appropriate.

Sorry, should have put in the link: https://github.com/dotnet/csharplang/issues/7905

linkdotnet avatar Feb 22 '25 10:02 linkdotnet

Hey @Joy-less

I made a recent change (which will be v3 as there are smaller breaking changes) that will use params ReadOnlySpan<T>. Therefore thanks for the suggestion and I will close that ticket.

linkdotnet avatar May 03 '25 09:05 linkdotnet