A doubt with the ICloneable interface in petshop 4.0 ?

Question

In the 'DBUtility' project of Petshop 4.0,the abstract class SqlHelper has a method 'GetCachedParameters':

        public static SqlParameter[] GetCachedParameters(string cacheKey) {
        SqlParameter[] cachedParms = (SqlParameter[])parmCache[cacheKey];

        if (cachedParms == null)
            return null;

        SqlParameter[] clonedParms = new SqlParameter[cachedParms.Length];

        for (int i = 0, j = cachedParms.Length; i < j; i++)
            clonedParms[i] = (SqlParameter)((ICloneable)cachedParms[i]).Clone();

        return clonedParms;
    }

why not return the 'cachedParms' directly ?

Answer 1

If cachedParms were returned directly, the caller could then change the elements of the array. The contents of the cache would then be effectively corrupted - the next caller to fetch the parameters from the cache with the same cache key would get unexpected results.

EDIT: Cloning the array itself prevents the elements being replaced with different parameters. Cloning the elements as well prevents the parameter objects being mutated. Basically it's all defensive coding.

Answer 2

To add to what Jon Skeet said, if the return'd Cache'd values are used internally for then you don't want the user to be using values which could change without them being aware.