Skip to content

Conversation

@SadToBeNep
Copy link
Contributor

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

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.

@dragon7307 dragon7307 added this to the Version 1.3.0 milestone Feb 7, 2024
@SadToBeNep
Copy link
Contributor Author

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

Guiorgy commented Feb 7, 2024

@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 dragon7307 changed the title Implemented Min() function and updated README.md Implemented Min() & Max() functions and updated README.md Feb 8, 2024
@dragon7307
Copy link
Member

dragon7307 commented Feb 8, 2024

Great work - I'll merge it into a feature branch. I believe there are some missing overloads and implementing MinBy and MaxBy probably 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 dragon7307 changed the base branch from main to feature-Min-Max February 8, 2024 14:15
@dragon7307
Copy link
Member

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

@dragon7307 dragon7307 merged commit f6822b1 into draconware-dev:feature-Min-Max Feb 8, 2024
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.

3 participants