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 Enumerable.Reverse(TSource[]) #107957

Merged
merged 4 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libraries/System.Linq.Queryable/tests/Queryable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ public static void MatchSequencePattern()
nameof(Enumerable.Prepend),
nameof(Enumerable.ToHashSet),
nameof(Enumerable.TryGetNonEnumeratedCount),
nameof(Enumerable.Reverse),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish there was a simple way to distinguish between the different overloads, but I suppose this should suffice.

"Fold",
"LeftJoin",
}
Expand Down
1 change: 1 addition & 0 deletions src/libraries/System.Linq/ref/System.Linq.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ public static System.Collections.Generic.IEnumerable<
public static System.Collections.Generic.IEnumerable<int> Range(int start, int count) { throw null; }
public static System.Collections.Generic.IEnumerable<TResult> Repeat<TResult>(TResult element, int count) { throw null; }
public static System.Collections.Generic.IEnumerable<TSource> Reverse<TSource>(this System.Collections.Generic.IEnumerable<TSource> source) { throw null; }
public static System.Collections.Generic.IEnumerable<TSource> Reverse<TSource>(this TSource[] source) { throw null; }
public static System.Collections.Generic.IEnumerable<TResult> SelectMany<TSource, TResult>(this System.Collections.Generic.IEnumerable<TSource> source, System.Func<TSource, System.Collections.Generic.IEnumerable<TResult>> selector) { throw null; }
public static System.Collections.Generic.IEnumerable<TResult> SelectMany<TSource, TResult>(this System.Collections.Generic.IEnumerable<TSource> source, System.Func<TSource, int, System.Collections.Generic.IEnumerable<TResult>> selector) { throw null; }
public static System.Collections.Generic.IEnumerable<TResult> SelectMany<TSource, TCollection, TResult>(this System.Collections.Generic.IEnumerable<TSource> source, System.Func<TSource, System.Collections.Generic.IEnumerable<TCollection>> collectionSelector, System.Func<TSource, TCollection, TResult> resultSelector) { throw null; }
Expand Down
15 changes: 15 additions & 0 deletions src/libraries/System.Linq/src/System/Linq/Reverse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,21 @@ public static IEnumerable<TSource> Reverse<TSource>(this IEnumerable<TSource> so
return new ReverseIterator<TSource>(source);
}

public static IEnumerable<TSource> Reverse<TSource>(this TSource[] source)
{
if (source is null)
{
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
}

if (source.Length == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I would probably just have this method call the existing Reverse directly, although I see why you might have wanted to avoid a redundant type check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, my thought process was "I could just call Reverse, but it's not like this a complicated method or changing very often, and this is more efficient".

{
return [];
}

return new ReverseIterator<TSource>(source);
}

/// <summary>
/// An iterator that yields the items of an <see cref="IEnumerable{TSource}"/> in reverse.
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions src/libraries/System.Linq/tests/ConsistencyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ private static IEnumerable<string> GetExcludedMethods()
nameof(Enumerable.ToList),
nameof(Enumerable.ToHashSet),
nameof(Enumerable.TryGetNonEnumeratedCount),
nameof(Enumerable.Reverse),
"Fold",
"LeftJoin",
};
Expand Down
37 changes: 36 additions & 1 deletion src/libraries/System.Linq/tests/ReverseTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ public class ReverseTests : EnumerableTests
[Fact]
public void InvalidArguments()
{
AssertExtensions.Throws<ArgumentNullException>("source", () => Enumerable.Reverse<string>(null));
AssertExtensions.Throws<ArgumentNullException>("source", () => Enumerable.Reverse<string>((IEnumerable<string>)null));
AssertExtensions.Throws<ArgumentNullException>("source", () => Enumerable.Reverse<string>((string[])null));
}

[Theory]
Expand Down Expand Up @@ -48,6 +49,40 @@ public void Reverse<T>(IEnumerable<T> source)
Assert.Equal(actual, actual); // Repeat the enumeration against itself.
}

[Theory]
[MemberData(nameof(ReverseData))]
public void ReverseArray<T>(IEnumerable<T> source)
{
T[] expected = source.ToArray();
Array.Reverse(expected);

IEnumerable<T> actual = source.ToArray().Reverse();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this test exercise the case where source is empty?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, an empty array is in the input data.


Assert.Equal(expected, actual);
Assert.Equal(expected.Count(), actual.Count()); // Count may be optimized.
Assert.Equal(expected, actual.ToArray());
Assert.Equal(expected, actual.ToList());

Assert.Equal(expected.FirstOrDefault(), actual.FirstOrDefault());
Assert.Equal(expected.LastOrDefault(), actual.LastOrDefault());

for (int i = 0; i < expected.Length; i++)
{
Assert.Equal(expected[i], actual.ElementAt(i));

Assert.Equal(expected.Skip(i), actual.Skip(i));
Assert.Equal(expected.Take(i), actual.Take(i));
}

Assert.Equal(default(T), actual.ElementAtOrDefault(-1));
Assert.Equal(default(T), actual.ElementAtOrDefault(expected.Length));

Assert.Equal(expected, actual.Select(_ => _));
Assert.Equal(expected, actual.Where(_ => true));

Assert.Equal(actual, actual); // Repeat the enumeration against itself.
}

[Theory, MemberData(nameof(ReverseData))]
public void RunOnce<T>(IEnumerable<T> source)
{
Expand Down
Loading