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 a ConcatAll<T>(IEnumerable<IEnumerable<T>>) method #556

Closed
bladeoflight16 opened this issue Jan 9, 2019 · 3 comments
Closed

Add a ConcatAll<T>(IEnumerable<IEnumerable<T>>) method #556

bladeoflight16 opened this issue Jan 9, 2019 · 3 comments

Comments

@bladeoflight16
Copy link

bladeoflight16 commented Jan 9, 2019

A commonly encountered scenario it to have an IEnumerable<IEnumerable<T>> that you wish to combine
into a single IEnumerable<T> without any transformations.

To do this using only the standard library, you can use SelectMany:

IEnumerable<SomeOtherClass> myOtherThings = GetMyOtherThings();
IEnumerable<MyT> myThings = myOtherThings.Select(o => o.GetMyTs()).SelectMany(t => t);

MoreLINQ provides a Flatten method to combine a sequence of arbitrarily nested sequences, but the fact it's not type safe creates even more boilerplate and turns type errors into runtime ones instead of compile time:

IEnumerable<SomeOtherClass> myOtherThings = GetMyOtherThings();
IEnumerable<MyT> myThings = myOtherThings.Select(o => o.GetMyTs()).Flatten().Cast<MyT>();

It would also go down too many levels if T happens to implement IEnumerable, though this is uncommon in my use cases at least.

It'd be much easier to read and understand with a ConcatAll method:

IEnumerable<SomeOtherClass> myOtherThings = GetMyOtherThings();
IEnumerable<MyT> myThings = myOtherThings.Select(o => o.GetMyTs()).ConcatAll();
@bladeoflight16 bladeoflight16 changed the title Add a ConcatAll<T>(IEnumerable<IEnumerable<T>>) method Add a ConcatAll<T>(IEnumerable<IEnumerable<T>>) method Jan 9, 2019
@atifaziz
Copy link
Member

atifaziz commented Jan 9, 2019

Since, as you said, there's SelectMany for this, the only value proposition of ConcatAll is to avoid the lambda? If that's the case then ConcatAll is too trivial to make it worthwhile for inclusion in a library.

If you think I've misunderstood your value proposition then happy to reconsider.

@atifaziz atifaziz closed this as completed Jan 9, 2019
@bladeoflight16
Copy link
Author

bladeoflight16 commented Jan 9, 2019

@atifaziz It's not just to avoid the lambda. It's to improve readability and make the code much easier to understand. I've had coworkers tell me that they look at SelectMany(i => i) and do not get "concatenate all" from it, and I tend to agree with that analysis. I don't want to have to explain it or worry about someone having to look up SelectMany to figure out what in the heck that code is doing. A good chunk of MoreLINQ could be implemented with out-of-the-box functions, but that would be harder to read and not as obvious what exactly the code is doing. Like Exclude can just be a combination of Take followed by Skip. This is the same kind of thing.

This is also similar to Prepend and Append. Aside from the extra clarity, all they really do is allow you to avoid creating a single item array or list to use Concat with.

@atifaziz
Copy link
Member

atifaziz commented Jan 11, 2019

If readability is the sole problem then it should be solved locally with a single-line aliasing wrapper:

public static IEnumerable<T> ConcatAll<T>(this IEnumerable<IEnumerable<T>> source) =>
    source.SelectMany(e => e);

While we may agree that SelectMany is not the best name (depending on what you're doing), neither is the other name it goes by: flat-map. Oh, and F# calls it collect. We risk running into he says to-may-to, she says to-mah-to? Doesn't change the fact that it's still a 🍅. 😉

A good chunk of MoreLINQ could be implemented with out-of-the-box functions, but that would be harder to read and not as obvious what exactly the code is doing.

To some extent but by that argument alone, you can eventually reduce just about everything in LINQ to Aggregate. MoreLINQ doesn't exist for the purpose of making LINQ code more readable. Most operators hopefully do something operationally intelligent.

Like Exclude can just be a combination of Take followed by Skip. This is the same kind of thing.

Exclude is not a combination of Take then Skip. Try it. It might seem the same kind of thing but it's not exactly the same thing. ConcatAll on the other hand would be exactly the same as SelectMany(i => i).

This is also similar to Prepend and Append. Aside from the extra clarity, all they really do is allow you to avoid creating a single item array or list to use Concat with.

Prepend and Append do a lot more. Have a look at the implementation. They are not just pretty helpers. Well, historically they were but not anymore. We changed that to optimize consecutive invocations of either. See PR #319 for background to this.

Also something that might not be obvious is the maintainer's dilemma: the trouble of allowing one-liners for purpose of readability means that many such requests will start pouring in and I fear that discussions will end up getting very subjective. The primary objectivity I'd like to maintain is therefore that a suggestion must be operationally intelligent or do some amount of non-trivial work. And even with that, we've struggled with the naming of many operators in MoreLINQ because one name might speak more to some people than another.

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

No branches or pull requests

2 participants