8

I have a question about CA1819 msdn performance warning.

The rule is: Arrays returned by properties are not write-protected, even if the property is read-only. To keep the array tamper-proof, the property must return a copy of the array. Typically, users will not understand the adverse performance implications of calling such a property. Specifically, they might use the property as an indexed property.

I understand the rule.

I want to know if this happens only with arrays? If yes, why?

2 Answers 2

7

Arrays are reference types. That is when a property returns an array object it's returning a pointer/reference to it's internal array. The caller of the property can now modify that internal array (as they have a pointer to it), this is generally unwanted. To stop this you can return a copy of the internal array but now your doing a memory allocation and copy every time the property is used, and given that properties are meant to be light-weight that's not good.

This happens with any reference type that is mutable. If you have a private reference to a mutable reference type (Like a Stream, List or Dictionary) and you return that reference in a property than the caller can modify your data. If you have a value type (structs in C#) than they are always copied when they're returned from the property so the caller can't then modify your internal version of the data.

9
  • 2
    I would only note that making a copy of the array only protects the original array from tampering, not the new one. See also stackoverflow.com/a/210444 Sep 9, 2013 at 15:16
  • link. @ibasa. this article says that if collection are used (lists) then the problem can be fixed. And list is a reference type. Lets take an example 'code' private IDbCommand idbCommand;public IDbCommand Command { get { return idbCommand; } } when i try to change idbCommand from an outer class it does not allow me even though idCommand is a reference type. Thanks
    – Kriss
    Sep 9, 2013 at 15:19
  • You can just return IEnumerable<T> because all arrays implement this interface and you won't be able to modify it without casting.
    – brian
    Sep 9, 2013 at 15:30
  • 3
  • 1
    @Kriss it isn't just arrays, any reference type can be modified when returned from a property. Strings are immutable by the language definition, no other type is. You can certainly modify an IDbCommand if you return it from a property. The warning is about the performance implications of returning a copy of an array through a property then some unsuspecting dev using it in a foreach thus causing a copy of the entire array on every step through the iteration.
    – stonemetal
    Sep 10, 2013 at 17:21
4

Well, first off, .net has introduced IReadOnlyList and IReadOnlyDictionary which address this problem for generics(which is preferable to a base array), previously you would have to had created your own interface in order to see the same performance with the same restriction.

The basic problem, and no, it's not limited to arrays, is that you want to provide a method of accessing a bunch of items without requiring that they be iterated, but you don't want the consumer of your API to be adding or deleting items -- but previously all of the built in interfaces that allow what you want, also allowed what you don't want.

The suggested solution is both inefficient (creating a clone is slow) and misleading -- the consumer gets an array that he thinks he can modify and have that effect the object that returned it. Ie he thinks if he adds an element and then calls the property again, he will get an array with p+1 items. He also expects that o.ItemArray.Equals(o.ItemArray). Neither of these expectations will be true.

As a practical problem, this is no longer a problem if you can use the right version of the framework and something other than an array.

As for the performance problem...

if(o.ItemArray.Equals(o.ItemArray) || o.ItemArray.Equals(o2.ItemArray))

Creates 4 copies of a potentially large array.

Edit in response to comment:

This is not a problem specific to Arrays. List and Dictionaries are also mutable.

IList<string> lst = new List<string>();
lst.Add("hello");
lst[0]="bye";
Console.WriteLine(lst[0]);

And the above code sample shows the problems with mutable list as properties. Suppose that I had a class, Greetings, which had a list of words and phrases that you would use upon meeting someone. Hello, Hi, Hola, Hey dude, Whats up! Como esta and so forth. So, I have a property Words, and return the above lst. You can then immediately execute the 3rd line and change my "hello" to "bye" --for everyone that has a reference to that instance of the class.

But as I said, in 4.5, you would get around this by having your list be IReadOnlyLst and, using this...

return (IReadOnlyList<string>) lst;
13
  • To have to make a copy of an array because arrays are mutable is one of the many defects of C# in my humble opinion. Default behavior in a programming language should be efficient, not inefficient.
    – Neil
    Sep 9, 2013 at 15:58
  • @Neil There are probably many performance-intensive C#, or C++, programs that use warning-inducing code all over the place, simply because of they're aware of the risks and know how to use them properly. Unless I've misinterpreted the warning mentioned, and it actually refuses to return the same-copy array, even with a warning.
    – Katana314
    Sep 9, 2013 at 16:04
  • @Neil Is there any specific reason why only arrays can be modified? Other objects like string,list, IDbCommand etc. are not mutable. I think there is something that makes arrays mutable and other objects immutable, and that's what i am asking, do anyone know that reason?
    – Kriss
    Sep 10, 2013 at 7:34
  • @Kriss It was a decision made when they were determining how C# would work. I'm sure there are other objects which can be modified as well. I suppose the main reason for making arrays mutable was to offer both options to the programmer. I don't have a problem with alternatives, but the "default" should be the more efficient version in my humble opinion.
    – Neil
    Sep 10, 2013 at 7:55
  • @jmoreno Yes you are right, lists are mutable too by index. But why can't we do the same thing with strings? What is the reason that string are not mutable? Can we say that only objects that can be accessed by index(array,list,etc) are mutable?
    – Kriss
    Sep 10, 2013 at 8:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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