Removing n elements from array starting from index

Question

This method removes n elements starting from a given index, from an array of a given type. If n is positive it removes elements forwards, if it is negative it removes them backwards (e.g. for an array {1,2,3,4,5} Remove(2,2) results in {1,2,5}; Remove (2,-2) results in {1,4,5})

It there a better way to do it?

public static class ArrayExtensions
{
    public static T[] RemoveAt<T>(this T[] array, int idx, int len)
    {
        T[] newArray;

        if (len > 0)
        {
            if (idx + len > array.Length)
                len = array.Length - idx;

            newArray = new T[array.Length - len];
            if (idx > 0)
                Array.Copy(array, 0, newArray, 0, idx);

            if (idx < array.Length - 1)
                Array.Copy(array, idx + len, newArray, idx, array.Length - idx - len);
        }
        else
        {
            newArray = new T[array.Length + len];
            if (idx > 0)
                Array.Copy(array, 0, newArray, 0, idx + len);

            if (idx < array.Length - 1)
                Array.Copy(array, idx, newArray, idx + len, array.Length - idx);
        }            

        return newArray;
    }
}

Answer 1

I believe that if the len < 0, it's better to adjust idx and len values. This will eliminate two branches of code.

Another suggestion is to use more readable variable names.
For instance, method arguments could be named startIndex and length respectively.

public static T[] RemoveAt<T>(this T[] array, int startIndex, int length)
{
    if (array == null)
        throw new ArgumentNullException("array");

    if (length < 0)
    {
        startIndex += 1 + length;
        length = -length;
    }

    if (startIndex < 0)
        throw new ArgumentOutOfRangeException("startIndex");
    if (startIndex + length > array.Length)
        throw new ArgumentOutOfRangeException("length");

    T[] newArray = new T[array.Length - length];

    Array.Copy(array, 0, newArray, 0, startIndex);
    Array.Copy(array, startIndex + length, newArray, startIndex, array.Length - startIndex - length);

    return newArray;
}

Answer 2

On top of @Dmitry's answer

This method wouldn't be easy to use IMO. Having the ability to pass a negative length is just not natural.

In addition you didn't validate the methods argument enough. Assume a passed in array which is having only one item is used with a len ==-2 this would just blow in your face with an exception.

Answer 3

My answer is similar to Dmitry's, but I will use LINQ and check the original startIndex and length. I prefer to not mutate things if possible, especially parameters, even if they're value types, so that it's consistent with how you treat reference types.

public static T[] RemoveAt<T>(this T[] array, int startIndex, int length)
{
    if (array == null)
        throw new ArgumentNullException("array");

    if (startIndex < 0 || startIndex > array.Length - 1)
        throw new ArgumentOutOfRangeException("startIndex");
    if (startIndex + length < 0 || startIndex + length > array.Length)
        throw new ArgumentOutOfRangeException("length");

    int newStartIndex = length >= 0 ? startIndex : (startIndex + 1 + length);
    int newLength = Math.Abs(length);
    return array.Take(startIndex).Concat(array.Skip(startIndex + length)).ToArray();
}

That will throw an exception if any of the skipped elements are outside of the array. If you want, you can make your method like Take(), where it will remove the specified indexes in the array and ignore the specified indexes that are outside the array.

public static T[] RemoveAt<T>(this T[] array, int startIndex, int length)
{
    if (array == null)
        throw new ArgumentNullException("array");

    int newStartIndex = length >= 0 ? startIndex : (startIndex + 1 + length);
    int newLength = Math.Abs(length);
    if(newStartIndex < 0)
    {
        newLength = Math.Max(newLength - newStartIndex, 0);
        newStartIndex = 0;
    }
    return array.Take(startIndex).Concat(array.Skip(startIndex + length)).ToArray();
}

Answer 4

Well, I guess even without LINQ you could make several things better.

Code Logic suggestions

1) Regardless of the length's sign, the resulting array length is going to be the same. Thus this logic

    T[] newArray;
    if (len > 0)
    {
        newArray = new T[array.Length - len];
    }
    else
    {
        newArray = new T[array.Length + len];
    }

can be simplified to

    var newArray = new T[array.Length - Math.Abs(len)];

2) The code has kind of asymmetric parameter validation. You have the check for a positive case

    if (idx + len > array.Length)
        len = array.Length - idx;

but you do not have it for the negative case. In your example ({0,1,2,3,4}) I will see different behavior when calling

    array.RemoveAt(2, 4);
    array.RemoveAt(2, -4);

though actually doing similar things.

3) Also I would throw exceptions for all incorrect indexes and lengths. Look, here I am actually accessing similar non-existing place

    array.RemoveAt(4, 3);
    array.RemoveAt(6, 1);

but getting different behavior. First is considered to be OK, and second brings a .NET exception. Both reactions, imo, are not really nice.

4) Actually, I generally agree with Heslacher, negative length looks weird in C#. This feature is redundant because you can always just shift index in the method call. Simultaneously, logic becomes more complicated.

But if I desperately wanted to implement it, I would prefer to still have non-negative length and just add an enum parameter for direction with default value

    public static T[] RemoveAt<T>(
        this T[] array, 
        int idx, 
        int len, 
        Direction direction = Direction.Forward) 

Code style suggestions

1) I support other responders about naming here, and also I would prefer "count" instead of "length" as it is done in the similar Microsoft method. And, well, RemoveRange is clearer then RemoveAt.

2) Also you should add braces around if bodies, here is some reasoning.

3) Also you could create two little methods with filling the resulting array to improve readability, e.g. FillLeftHalf() & FillRightHalf().

Answer 5

like @Greg said in his comment - you can make it really short with LINQ and you don't have to check any ranges:

static IEnumerable<T> Remove<T>(this IEnumerable<T> items, int startIndex, int count)
{
    return
        count > 0
        ? items.TakeWhile((x, i) => i < startIndex).Concat(items.Skip(startIndex + count))
        : items.TakeWhile((x, i) => i < (startIndex + count + 1)).Concat(items.Skip(startIndex + 1));
}