Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add EquiInterleave #700

Closed
wants to merge 4 commits into from
Closed

Add EquiInterleave #700

wants to merge 4 commits into from

Conversation

Orace
Copy link
Contributor

@Orace Orace commented Nov 14, 2019

This PR addresses #695

MoreLinq/EquiInterleave.cs Outdated Show resolved Hide resolved
@Crossbow78
Copy link

Out of curiosity, what does this add compared to the existing Interleave operation, other than an exception with imbalanced sequences?

Wouldn't it be more elegant to make the second Interleave overload that accepts a ImbalancedInterleaveStrategy argument public, and add a "Throw" strategy to that enumeration? Or at least use it internally, since the enumeration logic should be identical?

@Orace
Copy link
Contributor Author

Orace commented Dec 5, 2019

Out of curiosity, what does this add compared to the existing Interleave operation, other than an exception with imbalanced sequences?

Wouldn't it be more elegant to make the second Interleave overload that accepts a ImbalancedInterleaveStrategy argument public, and add a "Throw" strategy to that enumeration? Or at least use it internally, since the enumeration logic should be identical?

The "strategies as parameter" pattern has been discussed for the zip methods. (I will try to find the reference later).
Turns out that "strategies as overload" pattern has been chosen for the zip methods : ZipShortest, ZipLongest and EquiZip.
I just kept this pattern here.

We can just add an overload that call the private parametrized method, but it turns out that the current "parametrizable" implementation has issue #694.

I prefer to split it. Like it's done here #715 for zip.

@atifaziz atifaziz changed the title Add EquiInterleave Add EquiInterleave Jan 20, 2023
@atifaziz
Copy link
Member

Closing as #695 was closed as unplanned for now.

@atifaziz atifaziz closed this Jan 20, 2023
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