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

Implement Linq.single #11

Merged
merged 1 commit into from Oct 27, 2017

Conversation

Projects
None yet
2 participants
@bsinky
Copy link
Collaborator

bsinky commented Oct 26, 2017

Should satisfy #8, with the exception that single as implemented here works like .NET's System.Linq.Single, whereas I see that first in haxesharp is more like System.Linq.FirstOrDefault.

If System.Linq.SingleOrDefault is the desired behavior for single here, that should be a small matter of removing some exceptions and the related unit tests.

Documentation for this can be generated just prior to this PR being merged, or even after, if need be.

@nightblade9

This comment has been minimized.

Copy link
Owner

nightblade9 commented Oct 27, 2017

I'm not sure if your experience is more using Linq-to-SQL and friends; what I'm really after is the system.linq.enumerable LINQ methods.

@nightblade9
Copy link
Owner

nightblade9 left a comment

Looks excellent overall, needs a bit of tweaking.

{
if (array == null)
{
throw new ArgumentNullException('"array" argument cannot be null.');

This comment has been minimized.

Copy link
@nightblade9

nightblade9 Oct 27, 2017

Owner

I think you can remove this case. I don't think it's possible to get a null array on an extension method. You would have to invoke something like var x:Array<T> = null; x.single. If that's possible. ugh, we have to add this to the other Linq methods too! (I forgot about this case.)

This comment has been minimized.

Copy link
@bsinky

bsinky Oct 27, 2017

Author Collaborator

Yeah, assuming the same semantics apply to extension methods in Haxe as in C#, which in this case it seems they do, the object from which the extension method is called could be null.

{
return array[0];
}
else if (array.length == 0)

This comment has been minimized.

Copy link
@nightblade9

nightblade9 Oct 27, 2017

Owner

Nevermind.

}
else
{
throw new InvalidOperationException('"array" length cannot be greater than 1.');

This comment has been minimized.

Copy link
@nightblade9

nightblade9 Oct 27, 2017

Owner

Actually, it can be one; can call [1, 2, 3].first((n) => n == 1. This should be removed.

This comment has been minimized.

Copy link
@bsinky

bsinky Oct 27, 2017

Author Collaborator

I was actually following the cases where .NET's Enumerable.Single<TSource> Method (IEnumerable<TSource>) throws exceptions, as shown here: https://msdn.microsoft.com/en-us/library/bb155325(v=vs.110).aspx#Anchor_1

But if that's not useful for us to have in haxesharp, I can remove that. If that's the case, we probably wouldn't need the option to call single without a predicate at all, and could make that argument non-optional.

This comment has been minimized.

Copy link
@nightblade9

nightblade9 Oct 27, 2017

Owner

Ah, I didn't realize this was in an if (predicate == null) statement. Disregard my comment please.

Assert.that([1,2,3].single((x) => x == 3), Is.equalTo(3));
}

@Test

This comment has been minimized.

Copy link
@nightblade9

nightblade9 Oct 27, 2017

Owner

Your test coverage is excellent, well done. I would recommend adding a case to trigger when the array is null (that check I commented on earlier).

This comment has been minimized.

Copy link
@bsinky

bsinky Oct 27, 2017

Author Collaborator

The think the test singleThrowsIfArrayIsNull I added covers this? I could split it into two tests, one for single with predicate and one for single without predicate if that seems cleaner.

This comment has been minimized.

Copy link
@nightblade9

nightblade9 Oct 27, 2017

Owner

Ah, sorry, I didn't notice that. That's fine.

@nightblade9 nightblade9 merged commit 69dc066 into nightblade9:master Oct 27, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@bsinky bsinky deleted the bsinky:add-linq-single branch Oct 27, 2017

@nightblade9 nightblade9 referenced this pull request Oct 29, 2017

Closed

Need LINQ's .Single #8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.