CA2227: Collection properties should be read only

• 09/28/2018
• 3 minutes to read
• Contributors
  • 
  • 
  • 
  • 
  • 

Cause

An externally visible, writable property is of a type that implements System.Collections.ICollection. This rule ignores arrays, indexers (properties with the name 'Item'), and permission sets.

Rule description

A writable collection property allows a user to replace the collection with a completely different collection. A read-only property stops the collection from being replaced, but still allows the individual members to be set. If replacing the collection is a goal, the preferred design pattern is to include a method to remove all the elements from the collection, and a method to repopulate the collection. See the Clear and AddRange methods of the System.Collections.ArrayList class for an example of this pattern.

Both binary and XML serialization support read-only properties that are collections. The System.Xml.Serialization.XmlSerializer class has specific requirements for types that implement ICollection and System.Collections.IEnumerable in order to be serializable.

How to fix violations

To fix a violation of this rule, make the property read-only. If the design requires it, add methods to clear and repopulate the collection.

When to suppress warnings

You can suppress the warning if the property is part of a Data Transfer Object (DTO) class.

Otherwise, do not suppress warnings from this rule.

Example

The following example shows a type with a writable collection property and shows how the collection can be replaced directly. Additionally, it shows the preferred manner of replacing a read-only collection property using Clear and AddRange methods.

using System.Collections;

namespace csharp_code_analysis_examples
{
    public class WritableCollection
    {
        public ArrayList SomeStrings
        {
            get;

            // This set accessor violates rule CA2227.
            // To fix the code, remove this set accessor.
            set;
        }

        public WritableCollection()
        {
            SomeStrings = new ArrayList(new string[] { "one", "two", "three" });
        }
    }

    class ReplaceWritableCollection
    {
        static void Main()
        {
            ArrayList newCollection = new ArrayList(new string[] { "a", "new", "collection" });

            WritableCollection collection = new WritableCollection();

            // This line of code demonstrates how the entire collection
            // can be replaced by a property that's not read only.
            collection.SomeStrings = newCollection;

            // If the intent is to replace an entire collection,
            // implement and/or use the Clear() and AddRange() methods instead.
            collection.SomeStrings.Clear();
            collection.SomeStrings.AddRange(newCollection);
        }
    }
}

Public Class WritableCollection

    ' This property violates rule CA2227.
    ' To fix the code, add the ReadOnly modifier to the property:
    ' ReadOnly Property SomeStrings As ArrayList
    Property SomeStrings As ArrayList

    Sub New()
        SomeStrings = New ArrayList(New String() {"one", "two", "three"})
    End Sub

End Class

Class ViolatingVersusPreferred

    Shared Sub Main()
        Dim newCollection As New ArrayList(New String() {"a", "new", "collection"})

        Dim collection As New WritableCollection()

        ' This line of code demonstrates how the entire collection
        ' can be replaced by a property that's not read only.
        collection.SomeStrings = newCollection

        ' If the intent is to replace an entire collection,
        ' implement and/or use the Clear() and AddRange() methods instead.
        collection.SomeStrings.Clear()
        collection.SomeStrings.AddRange(newCollection)
    End Sub

End Class

#include "stdafx.h"
using namespace System;
using namespace System::Collections;

namespace UsageLibrary
{
    public ref class WritableCollection
    {
    public:
        property ArrayList^ SomeStrings
        {
            ArrayList^ get() { return someStrings; }

            // This set accessor violates rule CA2227.
            // To fix the code, remove this set accessor.
            void set(ArrayList^ value) { someStrings = value; }
        }

        WritableCollection()
        {
            someStrings = gcnew ArrayList(gcnew array<String^> {"one", "two", "three"});
        }

    private:
        ArrayList ^ someStrings;
    };
}

using namespace UsageLibrary;

void main()
{
    ArrayList^ newCollection = gcnew ArrayList(gcnew array<String^> {"a", "new", "collection"});

    WritableCollection^ collection = gcnew WritableCollection();

    // This line of code demonstrates how the entire collection
    // can be replaced by a property that's not read only.
    collection->SomeStrings = newCollection;

    // If the intent is to replace an entire collection,
    // implement and/or use the Clear() and AddRange() methods instead.
    collection->SomeStrings->Clear();
    collection->SomeStrings->AddRange(newCollection);
}

Related rules

• CA1819: Properties should not return arrays