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

Fix #74: typing of GroupBy overload with result selector. #75

Merged
merged 1 commit into from
May 15, 2018

Conversation

ergwun
Copy link
Contributor

@ergwun ergwun commented May 14, 2018

This pull request is for a fix for issue #74.

I have fixed the typing of the GroupBy overload with the result selector, and added a unit test to show successful assignation of that overload's result to an explicitly-typed variable.

It is also applicable to the linq-es5 branch, but I presume that would be handled independently.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.804% when pulling 6b711bc on ergwun:issue74 into e098395 on ENikS:master.

@ENikS
Copy link
Owner

ENikS commented May 15, 2018

I am not sure this is the best approach.
@MgSam tried to improve IGrouping in this commit but I guess he did not cover all the cases. Perhaps you could do something alone these lines?

@ergwun
Copy link
Contributor Author

ergwun commented May 15, 2018

I don't think it is a problem with IGrouping, as I do not believe that the third overload should be returning a grouping at all.

I'm basing this on the return types of the various GroupBy overloads in C#. Note how the first two overloads return enumerables of groupings, but the one with the result selector returns an enumerable of the type returned by the result selector:

public static IEnumerable<IGrouping<TKey, TSource>> GroupBy<TSource, TKey>(
	this IEnumerable<TSource> source,
	Func<TSource, TKey> keySelector
)

public static IEnumerable<IGrouping<TKey, TElement>> GroupBy<TSource, TKey, TElement>(
	this IEnumerable<TSource> source,
	Func<TSource, TKey> keySelector,
	Func<TSource, TElement> elementSelector
)

public static IEnumerable<TResult> GroupBy<TSource, TKey, TResult>(
	this IEnumerable<TSource> source,
	Func<TSource, TKey> keySelector,
	Func<TKey, IEnumerable<TSource>, TResult> resultSelector
)

@ENikS ENikS merged commit bf8e602 into ENikS:master May 15, 2018
@ergwun ergwun deleted the issue74 branch May 15, 2018 23:28
@MgSam
Copy link
Contributor

MgSam commented May 18, 2018

@ergwun Good catch, my mistake. Could you update the linq-es5 branch as well? (It's actually a different package on NPM)

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.

None yet

4 participants