CODEWITHSUNDEEP
c#arrays.net
Avner Hoffmann
Avner Hoffmann

Reputation: 421

Properties should not return arrays

Yes, I know this has been discussed many times before, and I read all the posts and comments regarding this question, but still can't seem to understand something.

One of the options that MSDN offers to solve this violation, is by returning a collection (or an interface which is implemented by a collection) when accessing the property, however clearly it does not solve the problem because most collections are not immutable and can also be changed.

Another possibility I've seen in the answers and comments to this question is to encapsulate the array with a ReadOnlyCollection and return it or a base interface of it(like IReadOnlyCollection), but I don't understand how this solves the performance issue.

If at any time the property is referenced it needs to allocate memory for a new ReadOnlyCollection that encapsulates the array, so what is the difference (in a manner of performance issues, not editing the array/collection) than simply returning a copy of the original array?

Moreover, ReadOnlyCollection has only one constructor with IList argument so there's a need to wrap the array with a list prior to creating it.

If I intentionally want to work with an array inside my class (not as immutable collection), is the performance better when I allocate new memory for a ReadOnlyCollection and encapsulate my array with it instead of returning a copy of the array?

Please clarify this.

Upvotes: 28

Views: 11757

Answers (3)

Nikolay Zlatev
Nikolay Zlatev

Reputation: 36

If I intentionally want to work with an array inside my class (not as immutable collection), is the performance better when I allocate new memory for a ReadOnlyCollection and encapsulate my array with it instead of returning a copy of the array?

not generally recommended, you can suppress a warning!

#pragma warning disable CA1819
// The code that's violating the rule is on this line.
#pragma warning restore CA1819

Upvotes: -1

Jon Skeet
Jon Skeet

Reputation: 1503859

If at any time the property is referenced it needs to allocate memory for a new ReadOnlyCollection that encapsulates the array, so what is the difference (in a manner of performance issues, not editing the array/collection) than simply returning a copy of the original array?

A ReadOnlyCollection<T> wraps a collection - it doesn't copy the collection.

Consider:

public class Foo
{
    private readonly int[] array; // Initialized in constructor

    public IReadOnlyList<int> Array => array.ToArray(); // Copy
    public IReadOnlyList<int> Wrapper => new ReadOnlyCollection<int>(array); // Wrap
}

Imagine your array contains a million entries. Consider the amount of work that the Array property has to do - it's got to take a copy of all million entries. Consider the amount of work that the Wrapper property has to do - it's got to create an object which just contains a reference.

Additionally, if you don't mind a small extra memory hit, you can do it once instead:

public class Foo
{
    private readonly int[] array; // Initialized in constructor
    private readonly IReadOnlyList<int> Wrapper { get; }

    public Foo(...)
    {
        array = ...;
        Wrapper = new ReadOnlyCollection<int>(array);
    }
}

Now accessing the Wrapper property doesn't involve any allocation at all - it doesn't matter if all callers see the same wrapper, because they can't mutate it.

Upvotes: 33

Dmitrii Bychenko
Dmitrii Bychenko

Reputation: 186833

You have no need to copy an array, just return it as IReadOnlyCollection<T>:

  public class MyClass {
    private int[] myArray = ...

    public IReadOnlyCollection<int> MyArray {
      get {
        return myArray;
      }
    }
  }

Upvotes: 5

Related Questions