morelinq / MoreLINQ

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

🚧 Add simpler `Unfold` overload #1015

Open atifaziz opened 8 months ago

atifaziz commented 8 months ago

This PR adds a simpler overload of Unfold that requires only a single function instead of 3. Example usage:

var xs = MoreEnumerable.Unfold(1, s => s <= 100 ? (true, s * 2, s) : default);
Console.WriteLine(string.Join(", ", xs));

Prints:

1, 2, 4, 8, 16, 32, 64

Pending work:

viceroypenguin commented 8 months ago

How is this different than MoreEnumerable.Generate(1, s => s *2).TakeWhile(s => s <= 100)?

codecov[bot] commented 8 months ago

Codecov Report

Merging #1015 (1559a27) into master (5eebc0a) will increase coverage by 0.00%. Report is 2 commits behind head on master. The diff coverage is 100.00%.

:exclamation: Current head 1559a27 differs from pull request most recent head 4fcd56f. Consider uploading reports for the commit 4fcd56f to get more accurate results

@@           Coverage Diff           @@
##           master    #1015   +/-   ##
=======================================
  Coverage   92.57%   92.58%           
=======================================
  Files         113      113           
  Lines        3422     3426    +4     
  Branches     1055     1057    +2     
=======================================
+ Hits         3168     3172    +4     
  Misses        191      191           
  Partials       63       63           
Files Coverage Δ
MoreLinq/Batch.cs 98.07% <ø> (ø)
MoreLinq/Unfold.cs 95.00% <100.00%> (+1.25%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

atifaziz commented 8 months ago

How is this different than MoreEnumerable.Generate(1, s => s *2).TakeWhile(s => s <= 100)?

That specific example is not, but Unfold anyway is Generate + TakeWhile + Select rolled into one. If TState and TResult are identical then Select can be dropped. Historically, Generate was there first, back in 2009. Unfold was added later in 2017. Either can be written in terms of the other.

For comparison, below is a slightly more involved example that generates the Fibonacci sequence where all numbers are less than 100. Using Generate + TakeWhile + Select (requires 3 lambdas):

var fibonacciNumbersLowerThan100 =
    MoreEnumerable.Generate((Curr: 0, Next: 1), s => (s.Next, s.Curr + s.Next))
                  .TakeWhile(s => s.Curr < 100)
                  .Select(s => s.Curr);

Using Unfold (requires 4 lambdas):

var fibonacciNumbersLowerThan100 =
    MoreEnumerable.Unfold((Curr: 0, Next: 1),
                          s => (s.Curr, Next: (Curr: s.Next, Next: s.Curr + s.Next)),
                          s => s.Curr < 100,
                          s => s.Next,
                          s => s.Curr);

Using the overload proposed here (requires 1 lambda):

var fibonacciNumbersLowerThan100 =
    MoreEnumerable.Unfold((Curr: 0, Next: 1),
                          s => s.Curr < 100 ? (true, (s.Next, s.Curr + s.Next), s.Curr) : default);
viceroypenguin commented 8 months ago

nit: for this example, this would be cleaner as it does not rely on a ternary or default

var fibonacciNumbersLowerThan100 =
    MoreEnumerable.Unfold((Curr: 0, Next: 1),
                          s => (s.Curr < 100, (s.Next, s.Curr + s.Next), s.Curr));

That said, I'm curious to whom you see this as a potential benefit.

atifaziz commented 8 months ago

nit: for this example, this would be cleaner as it does not rely on a ternary or default

Yes, but with the ternary (for illustration) and more generally, you avoid wasteful computation (like even s.Curr + s.Next) if not needed because the break condition's been met.

That said, I'm curious to whom you see this as a potential benefit.

People used to unfold from other languages and/or runtimes/libraries (such as someone used to Seq.unfold in F#) will find it more aligned and natural.

  • I don't see this version of Unfold (or even the original one) as simpler than Generate().TakeWhile().Select(); on the contrary, I'd argue that Generate() is more advantageous because it relies on composability which is both more flexible and more consistent in reading.

It's not trying to be simpler than Generate + TakeWhile + Select. It's overloading the existing Unfold with a simpler version.

  • I'm not sure that I see this version of Unfold as using less lambdas, because the implementation requires three lambdas, so it is still a four-lambda solution in the end.

The implementation is immaterial as it can change so the point I was makign is how many lambdas the developer has to author.

  • I might be able to buy a performance argument, but I'd have to see a benchmark to confirm the validity.

There's no performance argument being made for now.