Do not call properties that clone values in loops

Cause

A method calls the get accessor of a property inside an iteration statement, such as a for or while loop, and the accessor returns a cloned object.

Rule Description

A cloned object is a new object created with a value identical to that of an existing object. A cloned object is retrieved from a call to the System.ICloneable.Clone method, which creates a new instance of its declaring type with a value identical to the current instance.

This rule warns about a property accessed inside an iteration statement when the property returns a cloned object. In this case, multiple identical objects are created. If this is not the intent, access the property outside the iteration statement. Otherwise, multiple unnecessary objects are created and afterward, these objects must be garbage collected. This degrades performance, especially in compact iteration statements.

How to Fix Violations

To fix a violation of this rule, assign the property to a local variable outside the iteration statement and use the local variable inside the iteration statement.

When to Exclude Warnings

Exclude a warning from this rule if the intent is to create the cloned values inside the iteration statement. It is also safe to exclude a warning from this rule, or ignore the rule entirely, if performance is not a concern.

Example

The following example shows two violations of this rule, one of which should be excluded, and a method, BetterPerformingMethod, that satisfies the rule using a local variable to store the cloned value.

Imports System
Imports System.Collections

Namespace PerformanceLibrary

   Public Class PropertyThatClones
   
      Dim someList As New ArrayList()

      Readonly Property List As ArrayList
         Get
            Return DirectCast(someList.Clone(), ArrayList)
         End Get
      End Property

      Sub ModifyList(anything As Object)
         someList.Add(anything)
      End Sub

   End Class

   Public Class UseProperty
   
      Dim cloner As PropertyThatClones
      Dim arrayOfLists(10) As ArrayList 

      Sub New
         cloner = New PropertyThatClones()
         cloner.ModifyList("thirteen")
      End Sub

      ' The following method violates the rule.
      Sub UnderperformingMethod()
         For I As Integer = 0 To 9
            Console.WriteLine(cloner.List)
         Next 
      End Sub

      ' The following method satisfies the rule.
      Sub BetterPerformingMethod()
         Dim localList As ArrayList = cloner.List

         For I As Integer = 0 To 9
            Console.WriteLine(localList)
         Next 
      End Sub

      ' The following method violates the rule but the 
      ' violation should be excluded because the array 
      ' of identical ArrayList instances is intended.
      Sub BuildArrayOfArrayLists()
         For I As Integer = 0 To arrayOfLists.Length - 1
            arrayOfLists(i) = cloner.List
         Next
      End Sub

   End Class

End Namespace

using System;
using System.Collections;

namespace PerformanceLibrary
{
   public class PropertyThatClones
   {
      ArrayList someList = new ArrayList();

      public ArrayList List
      {
         get
         {
            return (ArrayList)someList.Clone();
         }
      }

      public void ModifyList(object anything)
      {
         someList.Add(anything);
      }
   }

   public class UseProperty
   {
      PropertyThatClones cloner;
      ArrayList[] arrayOfLists;

      public UseProperty()
      {
         cloner = new PropertyThatClones();
         cloner.ModifyList("thirteen");
         arrayOfLists = new ArrayList[10];
      }

      // The following method violates the rule.
      public void UnderperformingMethod()
      {
         for(int i = 0; i < 10; i++)
         {
            Console.WriteLine(cloner.List);
         }
      }

      // The following method satisfies the rule.
      public void BetterPerformingMethod()
      {
         ArrayList localList = cloner.List;

         for(int i = 0; i < 10; i++)
         {
            Console.WriteLine(localList);
         }
      }

      // The following method violates the rule but the 
      // violation should be excluded because the array 
      // of identical ArrayList instances is intended.
      public void BuildArrayOfArrayLists()
      {
         for(int i = 0; i < arrayOfLists.Length; i++)
         {
            arrayOfLists[i] = cloner.List;
         }
      }
   }
}

Related Rules

Do not concatenate strings inside loops