Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Add Array.Reverse<T> #7132

Merged
merged 2 commits into from
Sep 13, 2016
Merged

Add Array.Reverse<T> #7132

merged 2 commits into from
Sep 13, 2016

Conversation

justinvp
Copy link

@justinvp justinvp commented Sep 11, 2016

Add Array.Reverse<T> and update List<T>.Reverse to use it.

https://github.com/dotnet/corefx/issues/2352 (api-approved)

cc: @jkotas, @stephentoub, @weshaggard, @jamesqo, @benaadams

Add generic `Reverse<T>` methods to `Array`.
Implement `List<T>.Reverse` using `Array.Reverse<T>`.
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.array);
if (index < 0)
ThrowHelper.ThrowIndexArgumentOutOfRange_NeedNonNegNumException();
if (length < 0)
Copy link

Choose a reason for hiding this comment

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

This might increase the generated code size, but I think you can make this a teeny bit faster for the common case:

if ((index | length) < 0) // Will be true if either is negative
{
    if (index < 0)
        ThrowHelper.ThrowIndexArgumentOutOfRange_NeedNonNegNumException();
    else
        ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.length, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum);
}

Copy link
Author

Choose a reason for hiding this comment

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

I just aligned this with what @benaadams did for the non-generic Reverse in #7059.

@jamesqo
Copy link

jamesqo commented Sep 11, 2016

Once this is added to the contracts, there are other places that should probably be fixed to call Reverse, e.g. here

@jkotas
Copy link
Member

jkotas commented Sep 12, 2016

cc @janvorli

@janvorli
Copy link
Member

LGTM

@janvorli janvorli merged commit b33e756 into dotnet:master Sep 13, 2016
@jkotas
Copy link
Member

jkotas commented Sep 13, 2016

@justinvp Could you please port it to corert when you get a chance?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants