-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Added Enumerable.Append and Enumerable.Prepend #5947
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1152,6 +1152,30 @@ private static IEnumerable<TSource> ConcatIterator<TSource>(IEnumerable<TSource> | |
| foreach (TSource element in second) yield return element; | ||
| } | ||
|
|
||
| public static IEnumerable<TSource> Append<TSource>(this IEnumerable<TSource> source, TSource element) | ||
| { | ||
| if (source == null) throw Error.ArgumentNull("source"); | ||
| return AppendIterator<TSource>(source, element); | ||
| } | ||
|
|
||
| private static IEnumerable<TSource> AppendIterator<TSource>(IEnumerable<TSource> source, TSource element) | ||
| { | ||
| foreach (TSource e1 in source) yield return e1; | ||
| yield return element; | ||
| } | ||
|
|
||
| public static IEnumerable<TSource> Prepend<TSource>(this IEnumerable<TSource> source, TSource element) | ||
| { | ||
| if (source == null) throw Error.ArgumentNull("source"); | ||
| return PrependIterator<TSource>(source, element); | ||
| } | ||
|
|
||
| private static IEnumerable<TSource> PrependIterator<TSource>(IEnumerable<TSource> source, TSource element) | ||
| { | ||
| yield return element; | ||
| foreach (TSource e1 in source) yield return e1; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: it's fine, but why
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because "element" is already taken by the parameter :-) |
||
| } | ||
|
|
||
| public static IEnumerable<TResult> Zip<TFirst, TSecond, TResult>(this IEnumerable<TFirst> first, IEnumerable<TSecond> second, Func<TFirst, TSecond, TResult> resultSelector) | ||
| { | ||
| if (first == null) throw Error.ArgumentNull("first"); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using Xunit; | ||
|
|
||
| namespace System.Linq.Tests | ||
| { | ||
| public class AppendPrependTests : EnumerableTests | ||
| { | ||
| [Fact] | ||
| public void SameResultsRepeatCallsIntQueryAppend() | ||
| { | ||
| var q1 = from x1 in new int?[] { 2, 3, null, 2, null, 4, 5 } | ||
| select x1; | ||
|
|
||
| Assert.Equal(q1.Append(42), q1.Append(42)); | ||
| Assert.Equal(q1.Append(42), q1.Concat(new int?[] { 42 })); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void SameResultsRepeatCallsIntQueryPrepend() | ||
| { | ||
| var q1 = from x1 in new int?[] { 2, 3, null, 2, null, 4, 5 } | ||
| select x1; | ||
|
|
||
| Assert.Equal(q1.Prepend(42), q1.Prepend(42)); | ||
| Assert.Equal(q1.Prepend(42), (new int?[] { 42 }).Concat(q1)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void SameResultsRepeatCallsStringQueryAppend() | ||
| { | ||
| var q1 = from x1 in new[] { "AAA", String.Empty, "q", "C", "#", "!@#$%^", "0987654321", "Calling Twice" } | ||
| select x1; | ||
|
|
||
| Assert.Equal(q1.Append("hi"), q1.Append("hi")); | ||
| Assert.Equal(q1.Append("hi"), q1.Concat(new string[] { "hi" })); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void SameResultsRepeatCallsStringQueryPrepend() | ||
| { | ||
| var q1 = from x1 in new[] { "AAA", String.Empty, "q", "C", "#", "!@#$%^", "0987654321", "Calling Twice" } | ||
| select x1; | ||
|
|
||
| Assert.Equal(q1.Prepend("hi"), q1.Prepend("hi")); | ||
| Assert.Equal(q1.Prepend("hi"), (new string[] { "hi" }).Concat(q1)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void EmptyAppend() | ||
| { | ||
| int[] first = { }; | ||
| Assert.Single(first.Append(42), 42); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void EmptyPrepend() | ||
| { | ||
| string[] first = { }; | ||
| Assert.Single(first.Prepend("aa"), "aa"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void PrependNoIteratingSourceBeforeFirstItem() | ||
| { | ||
| var ie = new List<int>(); | ||
| var prepended = (from i in ie select i).Prepend(4); | ||
|
|
||
| ie.Add(42); | ||
|
|
||
| Assert.Equal(prepended, ie.Prepend(4)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this test should assert that
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The idea was primarily to catch attempts to "probe" the source for emptiness or something like that before yielding the prepended element. It could be a problem if the whole thing is used in TakeWhile and if fetching elements from the source is somehow observable. Considering how simple this operator is, this kind of changes are unlikely to happen unnoticed. I just added a testcase since I thought about the scenario while making changes.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that's throwing out ideas, not an objection. I'm still confused by
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Probably because System.Linq.Expressions doesn't have a P2P reference to System.Linq and is building against the bits in the package rather than what's live in the repo.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah. So it won't fail until after this is merged? I was pretty sure it failed before under similar circumstances.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will add the new operators to the exclusion list.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, I don't think they're a match, being akin to Repeat and Range in producing elements.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Modulo the query operators that are not extension methods on
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably open an issue to discuss that, but I'm inclined to think that there's no good way to universally do that across providers except by casting back to |
||
| } | ||
|
|
||
| [Fact] | ||
| public void ForcedToEnumeratorDoesntEnumeratePrepend() | ||
| { | ||
| var iterator = NumberRangeGuaranteedNotCollectionType(0, 3).Prepend(4); | ||
| // Don't insist on this behaviour, but check it's correct if it happens | ||
| var en = iterator as IEnumerator<int>; | ||
| Assert.False(en != null && en.MoveNext()); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ForcedToEnumeratorDoesntEnumerateAppend() | ||
| { | ||
| var iterator = NumberRangeGuaranteedNotCollectionType(0, 3).Append(4); | ||
| // Don't insist on this behaviour, but check it's correct if it happens | ||
| var en = iterator as IEnumerator<int>; | ||
| Assert.False(en != null && en.MoveNext()); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void SourceNull() | ||
| { | ||
| Assert.Throws<ArgumentNullException>("source", () => ((IEnumerable<int>)null).Append(1)); | ||
| Assert.Throws<ArgumentNullException>("source", () => ((IEnumerable<int>)null).Prepend(1)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Combined() | ||
| { | ||
| var v = "foo".Append('1').Append('2').Prepend('3').Concat("qq".Append('Q').Prepend('W')); | ||
|
|
||
| Assert.Equal(v.ToArray(), "3foo12WqqQ".ToArray()); | ||
|
|
||
| var v1 = "a".Append('b').Append('c').Append('d'); | ||
|
|
||
| Assert.Equal(v1.ToArray(), "abcd".ToArray()); | ||
|
|
||
| var v2 = "a".Prepend('b').Prepend('c').Prepend('d'); | ||
|
|
||
| Assert.Equal(v2.ToArray(), "dcba".ToArray()); | ||
| } | ||
| } | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding a few tests that use Append and Prepend multiple times, e.g. e.Append(1).Append(2).Append(3), as well as intermixed, e.g. e.Append(2).Prepend(1).Append(3), etc. You might also inject some Concats in there. I'm imaging at some point we might want to create some optimized paths for when Append, Prepend, and Concat are used repetitively.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to bump a version number in a .csproj somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if it comes from .csproj
is AssemblyVersion honored by the build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is now 4.0.2.0
I can make it 4.0.3.0 or 4.1.0.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for new APIs we bump minor version. @ericstj, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will make it 4.1.0 since it is a compatible addition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it should be 4.1, probably should have already be updated to 4.1. Note that when you do this you are going to break our internal builds. I see you have already merged so please make sure to handle the internal build issues.