docs
docs copied to clipboard
dotnet
CA1819 no reason to use collection over array when you want to allow the property to be modified?
Issue description
In the documentation for code analysis rule CA1819: Properties should not return arrays, under the heading Allow users to modify a property the documentation shows two examples (one bad, one good) saying that instead of having a property that returns an array, you should return a ReadOnly Collection<T>.
However, I don't see any reason why this rule is in place for this specific scenario (when you want a user to be able to modify the property). I think you could achieve the same thing with by making the array ReadOnly instead. This would also be clearer because it's clear that the property is fixed in size and you can't add or subtract elements from/to it like you can with a Collection<T>.
I want to note that before raising this issue here I have:
- asked a question on StackOverflow
- asked a question on the support forum site
No one has been able to provide an answer. If there is something wrong with my understanding, please let me know I will update all my questions on the various websites. However, right now I don't see the reason for the code analysis flagging this usage of the rule. It feels as if it's there just to make the rule a bit more generic.
Target framework
This is a general issue with documentation in the .NET Fundamentals and a code analysis rule.
Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.
- ID: 01cdba19-e564-4ba2-ac9a-1b4855ccf983
- Version Independent ID: af77a4b7-a2a7-b69a-9e26-42538011f34b
- Content: CA1819: Properties should not return arrays (code analysis) - .NET
- Content Source: docs/fundamentals/code-analysis/quality-rules/ca1819.md
- Product: dotnet-fundamentals
- GitHub Login: @gewarren
- Microsoft Alias: gewarren
@dceuinton The core idea of this rule is to discourage exposing mutable or cloned data structures through properties. It is a good API design principle to keep properties lightweight in terms of execution performance for it's getter/setter AND return immutable data to justify it being a property of the object. However, as you have mentioned, there are obviously corner cases where you still want to expose mutable data through properties, especially for internal consumers such as in tests. For such cases, it is best to return the data type that best suits your need - it could be an array or a Collection<T> type, as you prefer. If you choose an array, you will need to add a source suppression for each such property, which can be avoided by using the Collection<T>, but honestly either should be fine given that you have already made an explicit design decision for the API to return mutable data.
@mavasani thank you for your response and suggestions which help clarify the goal of the rule. I'll update my questions on the other forums.
@dceuinton The core idea of this rule is to discourage exposing mutable or cloned data structures through properties. It is a good API design principle to keep properties lightweight in terms of execution performance for it's getter/setter AND return immutable data to justify it being a property of the object. However, as you have mentioned, there are obviously corner cases where you still want to expose mutable data through properties, especially for internal consumers such as in tests. For such cases, it is best to return the data type that best suits your need - it could be an array or a
Collection<T>type, as you prefer. If you choose an array, you will need to add a source suppression for each such property, which can be avoided by using theCollection<T>, but honestly either should be fine given that you have already made an explicit design decision for the API to return mutable data.
Based on this response it seems like there should be 2 rules:
-
Performance. Warning about properties that allocate array copies (for tamper proofing the object). Basically the clone part of this response.
-
Design or Usage or Maintainability. To discourage exposing mutable array through properties.
CA1819 is currently in the Performance category. I think this is misleading because it is triggered even if the property doesn't clone the array. So there is no Performance impact. Should the rule be moved to Usage or Maintainability (or even Design) category instead?
@DmitryMak Can you open an issue in the https://github.com/dotnet/roslyn-analyzers repo for your question?