CA1819: Properties shouldn't return arrays - W

问题:

I encountered this FxCop rule before and wasn't really content with how to solve violations (thread1, thread2). I now have another case where I need to correct violations of the CA1819 kind.

Specifically, I have an algorithm-library that performs some analytic calculations on a curve (x,y), with a public "input object" like this:

public class InputObject
{
        public double[] X { get; set; }
        public double[] Y { get; set; }
        // + lots of other things well
}

This object's X and Y properties are used in hundreds of locations within library, typically using indexes. The input object is never altered by the algorithms, but actually it shouldn't matter if so. Also, .Length is called pretty frequently. It's a mathematical library, and double[] is kind of the standard data type in there. In any case, fixing CA1819 will require quite some work.

I thought about using List<double>, since Lists support indexing and are quite similar to arrays but I'm not sure whether this may slow down the algorithms or whether FxCop will be happy with those Lists.

What is the best option to replace these double[] properties?

回答1:

If it is read only to external consumer and consumer does not want to access it by index then the best is to have a public read only property of type IEnumerable<> with method accessors to add and remove, this way you will not have to expose your array to someone to mess with.

If you need to access the indexers then expose it as read only property of type IList<> and probably return a ReadOnly instance, with methods to add and remove.

This way you keep encapsulation of the internal list and allow consumer to access it in a read only way

回答2:

As your link suggests:

To fix a violation of this rule, either make the property a method or change the property to return a collection.

Using a collection such as a List should not have a significant impact on performance.

回答3:

The big problem here isn't really what your library does with the values (which is a potential problem, albeit a much more manageable one), but rather what callers might do with the values. If you need to treat them as immutable, then you need to ensure that a library consumer cannot change the contents after their original assignment. The easy fix here would be to create an interface that exposes all the array members that your library uses, then create an immutable wrapper class for an array that implements this interface to use in your InputObject class. e.g.:

public interface IArray<T>
{
    int Length { get; }

    T this[int index] { get; }
}

internal sealed class ImmutableArray<T> : IArray<T>
    where T : struct
{
    private readonly T[] _wrappedArray;

    internal ImmutableArray(IEnumerable<T> data)
    {
        this._wrappedArray = data.ToArray();
    }

    public int Length
    {
        get { return this._wrappedArray.Length; }
    }

    public T this[int index]
    {
        get { return this._wrappedArray[index]; }
    }
}

public class InputObject
{
    private readonly IArray<double> _x;
    private readonly IArray<double> _y;

    public InputObject(double[] x, double[] y)
    {
        this._x = new ImmutableArray<double>(x);
        this._y = new ImmutableArray<double>(y);
    }

    public IArray<double> X
    {
        get { return this._x; }
    }

    public IArray<double> Y
    {
        get { return this._y; }
    }

    //...
}

The elements in your "immutable" array contents would still be mutable if T is mutable, but at least you're safe for the double type.

回答4:

Sometime FxCop from my point of view exagerates.

It all depends on what you have to do, if you are writing a complex system where security and very clean code is required, you should returns a readonly version of that array. That is, cast the array as IEnumerable as suggests devdigital or use the good idea ImmutableArray of Mohamed Abed, that i prefer.

If your are writing software that require high performance... there is nothing better than an array for performances in C#. Arrays can be a lot more performant for iterating and reading.

If performances are really important I suggest you to ignore that warning. Is still legal, also if not too much clean, to return a readonly array.

for (int i = 0; i < array.Length; ++i) { k = array[i] + 1; }

This is very fast for big arrays in C#: it avoids array bounds check. It will perform very much as a C compiled code would do.

I always wished a "readonly array" type in C# :) but there is no hope to see it.