locked
Question about CA1819: PropertiesShouldNotReturnArrays RRS feed

  • Question

  • Heya folks, i have a property that returns a byte array

    /// <summary>
    ///
    Binary data of the ringtone.
    ///
    </summary>
    public byte
    [] BinaryData
    {
        get { return this
    ._binaryData; }
        set
        {
           
    this._binaryData = this.PropertySetter(value, "BinaryData") as byte
    [];
        }
    }

    now i get a warning saying i shouldn't return arrays from properties. Hmm. ok - the rational (if i have understood it correctly) is becuase the array is not write-protected and therefore not tamper-proof.

    So, when i make two methods (a getter and a setter) i get asked to make a property! :) haha. so, maybe i should return a collection instead of an array. If so, what type of read-only collection could i please use instead?

    Sunday, January 29, 2006 3:48 AM

Answers

  • I think that an array is the best thing to use here, especially if it is storing the bytes for a image. This makes it easy to write it out to a stream.  I would also consider avoiding cloning the array as well if you are not concerned about consumers of your class being able to modify the array.

    Sunday, January 29, 2006 6:20 AM
    Moderator
  • Pure Krome,

    The fix for this will be in the next release of FxCop 1.35.

    Regards

    David

    Wednesday, February 8, 2006 2:25 PM
    Moderator

All replies

  • This is actually a known bug with UsePropertiesWhereAppropriate firing on SetXXXX methods that contain a single array parameter. I'll raise this issue again internally and see if we can get this fixed.

    When converting the property to a method you should do something like this:



    public void SetBinaryData(byte[] value)
    {
       this._binaryData = this.PropertySetter(value, "BinaryData") as byte[];
    }
     
    public byte[] GetBinaryData()
    {
      return (byte[])this._binaryData.Clone();
    }

     

    However, if the array you're returning is large, for example, an image, and you are calling the method alot, this cloning could increase the memory pressure on your application. In this situation, you have a tradeoff between correctness and performance.

    Sunday, January 29, 2006 5:19 AM
    Moderator
  • Hi David, thanks for the very prompt reply.

    heh -> that is EXACTLY what i did (your two methods), including doing the 'as byte[]' - i always try and do safe casting (or whatever the correct term is).

    the help file does suggest using a collection instead of an array - any suggestions what type of collections could be used here?

    lastly -> the byte[] is definatly for images, actually :)

    Sunday, January 29, 2006 5:37 AM
  • I think that an array is the best thing to use here, especially if it is storing the bytes for a image. This makes it easy to write it out to a stream.  I would also consider avoiding cloning the array as well if you are not concerned about consumers of your class being able to modify the array.

    Sunday, January 29, 2006 6:20 AM
    Moderator
  • Just to complete this post, we've gone ahead and used an array (eg. byte[]) but have put a once off ingore attribute about this property to keep code analysis/fxcop happy :)

    like u said, this is a one of example of where i our requirement contradicts the general usage of this fxcop design issue.

     

    cheers :)

    Wednesday, February 8, 2006 10:27 AM
  • Pure Krome,

    The fix for this will be in the next release of FxCop 1.35.

    Regards

    David

    Wednesday, February 8, 2006 2:25 PM
    Moderator
  • Hello,

    even though this is a very old thread:

    This issue still (or again) exists in FxCop 10.0

    regards

    Thomas

    Wednesday, December 12, 2012 8:31 AM