Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ValueStringBuilder.Concat? #237

Open
Joy-less opened this issue Feb 20, 2025 · 5 comments
Open

Remove ValueStringBuilder.Concat? #237

Joy-less opened this issue Feb 20, 2025 · 5 comments
Milestone

Comments

@Joy-less
Copy link
Contributor

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.

@linkdotnet
Copy link
Owner

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

@Joy-less
Copy link
Contributor Author

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

@linkdotnet
Copy link
Owner

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 linkdotnet added this to the v3 milestone Feb 21, 2025
@Joy-less
Copy link
Contributor Author

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.

@linkdotnet
Copy link
Owner

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: dotnet/csharplang#7905

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants