search

morelinq / MoreLINQ

Extensions to LINQ to Objects
https://morelinq.github.io/
Apache License 2.0
3.63k stars 409 forks source link

Add `AggregateOrDefault` #1033

Closed skarllot closed 7 months ago

skarllot commented 8 months ago

Add the extension method AggregateOrDefault allowing to aggregate over empty collections by returning a default value.

codecov[bot] commented 8 months ago

Codecov Report

Merging #1033 (b000e92) into master (092a40d) will increase coverage by 0.04%. The diff coverage is 100.00%.

:exclamation: Current head b000e92 differs from pull request most recent head b81272e. Consider uploading reports for the commit b81272e to get more accurate results

@@            Coverage Diff             @@
##           master    #1033      +/-   ##
==========================================
+ Coverage   92.62%   92.67%   +0.04%     
==========================================
  Files         113      114       +1     
  Lines        3419     3442      +23     
  Branches     1055     1060       +5     
==========================================
+ Hits         3167     3190      +23     
  Misses        189      189              
  Partials       63       63
Files Coverage Δ
MoreLinq/AggregateOrDefault.cs 100.00% <100.00%> (ø)

:mega: Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

skarllot commented 8 months ago

@atifaziz yes, you are right. The only difference that I can see is less allocation (do not create DefaultIfEmptyIterator instance).

atifaziz commented 7 months ago

The only difference that I can see is less allocation (do not create DefaultIfEmptyIterator instance).

That's true, but it's going to be hard to avoid that with most LINQ compositions/chains that run deeper than a single call. With every addition to the project, one has to weigh the cost of maintaining the code, documentation and tests (often long after the original contributor may have moved on) and I find that this one isn't worth it on the count of avoiding one allocation. I wish you had opened an issue to declare your intent to add this so I could've avoided you the trouble of going through the effort of coding, testing and submitting a PR in the first place. I will close this now, but I hope this does not discourage you from contributing in the future especially because (and for what it's worth stating) this was a good quality PR.

skarllot commented 7 months ago

No problem. I thought to open a issue before, but I could use this PR as training to understand this project particularities for future contributions.