MoreLINQ icon indicating copy to clipboard operation
MoreLINQ copied to clipboard

WindowLeftAndRight

Open atifaziz opened this issue 6 years ago • 3 comments

I am forking this issue out of #627 as the discussion on WindowLeftAndRight was diverging from the subject of #627. Below is the quoted discussion thus far.

@Orace said:

@atifaziz is there any equivalent to WindowLeftAndRight available ?

@atifaziz said:

is there any equivalent to WindowLeftAndRight available ?

No, never needed it and no one ever asked for it. Do you have a use case?

@Orace said:

I was looking for it to improve it ;) I think a Window(int size, bool extendLeft = false, bool extendRight = false, bool extendBoth = false) overload is missing here.

The behavior will be

extendLeft |= extendBoth;
extendRight |= extendBoth;

The usages pretty straightforward.

@atifaziz said:

For dynamic behaviour? I think if someone needs that, they can achieve it like this:

var xs = Enumerable.Range(1, 5);
var windows =
    from f in new Func<IEnumerable<int>, int, IEnumerable<IList<int>>>[]
    {
        MoreEnumerable.Window,
        MoreEnumerable.WindowLeft,
        MoreEnumerable.WindowRight,
    }
    select f(xs, 3).Select(w => $"[{w.ToDelimitedString(", ")}]")
                   .ToDelimitedString(", ");
foreach (var ws in windows)
    Console.WriteLine(ws);
// output:
// [1, 2, 3], [2, 3, 4], [3, 4, 5]
// [1, 2, 3], [2, 3, 4], [3, 4, 5], [4, 5], [5]
// [1], [1, 2], [1, 2, 3], [2, 3, 4], [3, 4, 5]

@Orace said:

My proposal is Window(size, extendLeft: true) as an alias for WindowLeft(size), same thing for right. + we have a free WindowLeftAndRight. And all of this can fit in a single implementation.

It facilitate the cases where the choice is made at runtime: Window(size, extendRight: isTailNeed)

We have to discuss the name of the optional extendLeft, extendRight, extendBoth parameters.

atifaziz avatar Nov 01 '19 09:11 atifaziz

Four things:

  • Boolean values hinder readability (which can be somewhat offset by naming arguments or, better yet, introducing an enum).
  • Why do you need three Boolean flags? Isn't extendBoth == extendLeft && extendRight?
  • WindowLeft, WindowRight and Window are statically searchable names and algorithms (think Find All References).
  • Dynamic behaviour is rarely needed.

I'm fine with introducing a private method to share the implementation among the three but I am not convinced that needs to be surfaced on the public API.

atifaziz avatar Nov 01 '19 09:11 atifaziz

WindowLeft and WindowRight stay the recommended choice for static cases. Window(int size) is preserved.

We add possibility for dynamic choice. I do think that the best way to write it is the optional parameters. See :

source.Foo().Window(size, extendLeft: isHeadNeeded).Bar()

VS

source.Foo().Window(size, isHeadNeeded ? WindowBehavior.ExtendLeft : WindowBehavior.Strict).Bar()

The extendBoth parameter is a shortcut for

source.Foo().Window(size, isExtendNeeded, isExtendNeeded).Bar()

And is used like that:

source.Foo().Window(size, extendBoth: isExtendNeeded).Bar()

Orace avatar Nov 01 '19 10:11 Orace

We add possibility for dynamic choice

Why? Just for the sake of adding features and richness? Is there a compelling use case that can be demonstrated?

Keep in mind, this project's tagline reads:

This project enhances LINQ to Objects with extra methods, in a manner which keeps to the spirit of LINQ.

Dynamic behaviour is not something that keeps to the spirit of LINQ.

atifaziz avatar Nov 20 '19 15:11 atifaziz