search

draconware-dev / SpanExtensions.Net

SpanExtensions.Net aims to make ReadOnlySpan<T> and Span<T> more accessible and overall better integrated into the .Net Ecosystem, by providing alternatives for many missing Extension Methods for Span<T>, ranging from string.Split() over Enumerable.Skip() and Enumerable.Take() to an improved ReadOnlySpan<T>.IndexOf().
MIT License
17 stars 5 forks source link

Implemented Min() & Max() functions and updated README.md #13

Closed SadToBeNep closed 5 months ago

SadToBeNep commented 5 months ago

Hello, I was using your lib for one of my project, and it was missing the .Min() function, so I added it

I am just learning C# and Git(Hub) too, so go easy on me if something

PS: Wasn't sure if I should open a Issue before the pull request

dragon7307 commented 5 months ago

Hey, Glad you like the project. I will have a look at your contribution and then merge it.
Thank you also for opening a pull request - that makes maintaining an open source project a whole lot easier!!

Of course, Min cannot be implemented without its counterpart Max, so I'll see when I have time to get around to that. Since I'm planning to release version 1.3.0 at the end of february, this is when your contribution should get shipped into production.

PS: Opening an issue never hurts, but is unnecessary in this case, since your pr gives all needed information.

SadToBeNep commented 5 months ago

I also started to work on Max, most likely I will have time tomorrow and can push that too up here in another pr

Guiorgy commented 5 months ago

@SadToBeNep If you plan to implement Max as well, maybe instead of another PR, I'd suggest updating the current title an have this PR for both, since as dragon stated, it doesn't make sence to add one and not the other.

dragon7307 commented 5 months ago

Great work - I'll merge it into a feature branch. I believe there are some missing overloads and implementing MinBy and MaxByprobably won't hurt, so I will implement them. I will also adapt the code to the projects style (e.g. lowercase local variables etc. ). And maybe it makes sense to look at vectorization here, since .Net does this internally.

dragon7307 commented 5 months ago

Also for performance reasons an if construct should be used instead of a ternary expression,to avoid unnecessary reassignments.