1
\$\begingroup\$

I need to resize an IList<T>. I have written two versions and would like to have a review of those. I am particularly interested on which of the two is to be preferred but grateful for all other advice as well.

using System;
using System.Collections.Generic;
using System.Linq;

namespace Foo
{
    public static class IListExtensions
    {
        public static void Resize<T>(this IList<T> list, int size)
        {
            if (list == null)
                throw new ArgumentException(nameof(list));

            if (size < 0)
                throw new ArgumentException(nameof(size));

            while (list.Count > size)
                list.RemoveAt(list.Count - 1);
        }

        public static void Resize2<T>(this IList<T> list, int size)
        {
            if (list == null)
                throw new ArgumentException(nameof(list));

            if (size < 0)
                throw new ArgumentException(nameof(size));

            var tmp = list.Take(size).ToList();
            list.Clear();
            list.AddRange(tmp);
        }

        public static void AddRange<T>(this IList<T> list, IEnumerable<T> items)
        {
            if (list == null)
                throw new ArgumentException(nameof(list));

            if (items == null)
                throw new ArgumentException(nameof(items));

            foreach (var item in items)
            {
                list.Add(item);
            }
        }
    }

    class Program
    {
        public static void Main(string[] args)
        {
            IList<string> words = new List<string>() { "foo", "bar", "baz" };
            words.Resize(1);

            IList<string> words2 = new List<string>() { "foo", "bar", "baz" };
            words2.Resize2(1);
        }
    }
}
\$\endgroup\$
  • 4
    \$\begingroup\$ I need to resize an IList<T> - but why? It can resize itself and removing items from it with .RemoveAt(list.Count - 1) is not resizing. It all depends on it internal implementation which should not be any of your concerns. Do you have any problems with it or why are you resizing it? \$\endgroup\$ – t3chb0t Aug 29 '17 at 8:46
  • \$\begingroup\$ Really strange post. You can take a look at one of the IList<T> implementations – List<T> – on Reference Source and see that neither RemoveAt nor Clear don't resize internal array. Or "resizing" in your undestanding is just taking first n elements? \$\endgroup\$ – Maxim Aug 29 '17 at 9:18
  • 1
    \$\begingroup\$ @t3chb0t I have three words in IList<string> words and want a list with one word. Why not? How can a IList<T>resize itself? Why is removing items different from resizing? (Appart from that I should have called it Shrink instead.) I do not care about the internal implementation, I just want to resize from three words to one. \$\endgroup\$ – Micha Wiedenmann Aug 29 '17 at 9:55
  • 1
    \$\begingroup\$ @MichaWiedenmann I feel there's a language barrier here - what you refer to isn't necessarily resizing so much as just removing elements. (Technically that doesn't have to change the size of the IList<T>, but it's possible that for some scenarios it will.) \$\endgroup\$ – Der Kommissar Aug 29 '17 at 11:27
  • \$\begingroup\$ msdn.microsoft.com/en-us/library/y33yd2b5(v=vs.110).aspx \$\endgroup\$ – paparazzo Aug 29 '17 at 16:32
2
\$\begingroup\$

You should throw the appropriate exception:

if (list == null)
    throw new ArgumentNullException(nameof(list));

if (size < 0)
    throw new ArgumentOutOfRangeException(nameof(size));

In general remember that first argument of ArgumentException isn't the parameter name but the error message.

Both versions are equally good (or bad) unless you have a specific usage scenario and some performance requirements, I can't really comment on that from a generic point of view.

Just one note: you remove tail items from the list but there are good chances (it depends on the effective collection type you're using) that size in memory won't decrease (for example with List<T> you have to call TrimExcess(). What does it mean? That, maybe, Resize() isn't the most clear name ever because callers may expect a different behavior.

You may be inspired by LINQ implementation and check for common cases, for example List<T>.

public static void Resize<T>(this IList<T> list, int size)
{
    if (list == null)
        throw new ArgumentNullException(nameof(list));

    if (size < 0)
        throw new ArgumentOutOfRangeException(nameof(size));

    if (list is List<T> genericList)
    {
        genericList.RemoveRange(size, list.Count - size);
    }
    else
    {
        while (list.Count > size)
            list.RemoveAt(list.Count - 1);
    }
}

The same is true for AddRange() which doesn't need to be manually implemented when list is List<T>.

Edit: EBrown noted in comments that this optimization may be unnecessary (and then counterproductive because it greatly hurts readability); I agree, if you need it or not is specific to your usage scenario and you should decide after after a microbenchmark with real world data.

A very quick benchmark (release build, one warm-up round, average of 100 executions) gave this (measure is in number of ticks then compare them cum grano salis):

Benchmark for small number of items

And:

Benchmark for bigger number of items

Label "N to M" means "list of N elements trimmed to M element". Strings are 12 characters strings.

That said, performance is not always required or needed, especially when sacrificing readability.

\$\endgroup\$
  • \$\begingroup\$ I must say this whole resizing thing is very suspicious and highly questionable. \$\endgroup\$ – t3chb0t Aug 29 '17 at 8:48
  • \$\begingroup\$ Well, it's not that uncommon task to cut the tail of a list (especially when working with old code which has not been ported to be LINQ friendly). \$\endgroup\$ – Adriano Repetti Aug 29 '17 at 8:53
  • \$\begingroup\$ if size < count then genericList.Capacity = size; will error. It even says so in the docs. You could do genericList.RemoveRange(size, list.Count - size); instead though. \$\endgroup\$ – RobH Aug 29 '17 at 9:56
  • \$\begingroup\$ Tnx @RobH, you're right. Fixed. \$\endgroup\$ – Adriano Repetti Aug 29 '17 at 10:14
  • \$\begingroup\$ @AdrianoRepetti Personally, I think this is a bad solution, because now you've added unnecessary branching, and the method became more complex for something that may be a performance advantage, but also may not be. \$\endgroup\$ – Der Kommissar Aug 29 '17 at 11:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.