search

Cysharp / ZString

Zero Allocation StringBuilder for .NET and Unity.
MIT License
2.04k stars 150 forks source link

Added a test for joining HashSet values. #84

Closed Henr1k80 closed 1 year ago

Henr1k80 commented 1 year ago

Added a test for joining HashSet values. It fails because the overload Join<T>(string separator, params T[] values) is used, even though there exists overloads for both Join<T>(string separator, ICollection<T> values) and Join<T>(char separator, IEnumerable<T> values) I cannot explain this, haven't dived deeper into this. The fix is easy, add another overload, but I am not sure how you want to handle this. Is it a bug in the framework? Will it affect other types? If it is just another overload, I can easily include the fix in the pull request. In my code I just cast it to IEnumerable<> and it works.

Henr1k80 commented 1 year ago

I have just benchmarked the cast to IEnumerable<> vs. the call to AsEnumerable and the results are equal for .NET 6.0

Henr1k80 commented 1 year ago

Ok, the extension method .AsEnumerable() seems to be a barely measurable faster than the cast 👌 At least for v6 Program.cs https://gist.github.com/Henr1k80/d6f0404b48a547ca71e05e78b87061e6

github-actions[bot] commented 1 year ago

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 30 days.

hadashiA commented 1 year ago

Hello, thank you for the report. You make a good point.

However, as you mentioned, doing .AsEnumerable() and casting seems to be the solution...

We considered adding an overload simply for HashSet<T> as you suggested, but it seems difficult to cover similar support for all expected collections, so we decided to not add it.

Therefore, this PR will also be closed. Thank you.