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

Added a test for joining HashSet values. #84

Closed
wants to merge 1 commit into from

Conversation

Henr1k80
Copy link

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
Copy link
Author

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

@Henr1k80
Copy link
Author

Henr1k80 commented Jan 13, 2023

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
Copy link
Contributor

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.

@github-actions github-actions bot added the stale label Jul 13, 2023
@neuecc neuecc removed the stale label Aug 9, 2023
@hadashiA
Copy link
Contributor

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.

@hadashiA hadashiA closed this Sep 15, 2023
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

Successfully merging this pull request may close these issues.

None yet

3 participants