FxCop, Properties and arrays

[steve bull]

If I create an array, e.g. private int[] x, inside a parent class whose elements can be accessed by a child class - what is the
recommended way of doing this?

When I create a property for the array FxCop complains that properties should not return arrays. If I want to make the array
private, which FxCop insists I do as well, is the only way of accessing the array elements via a method?

See e.g. http://www.gotdotnet.com/team/fxcop...formance/PropertiesShouldNotReturnArrays.html

the FxCop explanation describes what not to do but there is no explanation of what should be done instead.

Having Googled this I see that quite a number of people give the wrong way to do it and nobody seems to deal with the problem -
which seems to me to be a pretty basic one.



Thanks,
Steve

 

[Guest]

Steve,

is the only way of accessing the array elements via a method?

Yes, is there any reason you don't want to use one in this case?


Mattias

 

[Guest]

you can also use the indexers for accessing elements of that array in the
child class. But for using indexer you should also define a property in your
base class which returns the current length of the array, so the in child
class you can iterate on the elements of that array.

i hope it solves your problem

 

[steve bull]

I would like to use FxCop because it is very good at uncovering some bugs and as MS appear to be behind it would seem to
me that if they don't want you to expose arrays to even child classes they should have a simple way of accessing them
via properties. Wouldn't methods add more overhead - especially when they are called many times?

Really I am just looking for the preferred method of doing this. Still learning at the moment.


Steve

 

[Bill Butler]

I would like to use FxCop because it is very good at uncovering some bugs and as MS appear to be
behind it would seem to
me that if they don't want you to expose arrays to even child classes they should have a simple
way of accessing them
via properties. Wouldn't methods add more overhead - especially when they are called many times?

Really I am just looking for the preferred method of doing this. Still learning at the moment.

Hi Steve,

Here is the argument that FxCop is using:

If you return an Array(even readonly) the contents can still be modified.
Thus, if you don't want the external world to modify YOUR internal array you should pass the world a
COPY of your array instead of Your actual Array. However, copies can be time consuming(or not,
depending on the size).
Now
People don't EXPECT properties to have side effects. They may put them in loops and call them
repeatedly. So it is a bad idea to, Say, access a Database in a property, or Read in a File, Or Copy
an Array.

All they are SUGGESTING is that instead of
public int[] Blah
{
get{return CopyMyArray();}
}

You should do this instead.
public int[] GetBlah()
{
return CopyMyArray();
}

The Amount of executed code is the same, but your intentions are more clear. An explicit GetBlah()
method says "I might take a while to do this".

Now, If you are the only person ever using the internal Array and you don't intend to ever modify
the Array outside of your class....You COULD break encapsulation and simply return a reference to
your array (I know...BLASPHEMY).
But If you choose to go this route, you should be aware of the reason for the rule in the first
place.

Good Luck
Bill

 

[steve bull]

unfortunately the indexer cannot have a name so the index is on the class as a whole. which pretty much seems like a
waste of time. MS don't appear to have spent too much time thinking about this.


thanks,
steve

you can also use the indexers for accessing elements of that array in the
child class. But for using indexer you should also define a property in your
base class which returns the current length of the array, so the in child
class you can iterate on the elements of that array.

i hope it solves your problem

If I create an array, e.g. private int[] x, inside a parent class whose elements can be accessed by a child class - what is the
recommended way of doing this?

When I create a property for the array FxCop complains that properties should not return arrays. If I want to make the array
private, which FxCop insists I do as well, is the only way of accessing the array elements via a method?

See e.g. http://www.gotdotnet.com/team/fxcop...formance/PropertiesShouldNotReturnArrays.html

the FxCop explanation describes what not to do but there is no explanation of what should be done instead.

Having Googled this I see that quite a number of people give the wrong way to do it and nobody seems to deal with the problem -
which seems to me to be a pretty basic one.



Thanks,
Steve

 

[Bill Butler]

unfortunately the indexer cannot have a name so the index is on the class as a whole. which pretty
much seems like a
waste of time. MS don't appear to have spent too much time thinking about this.

This is true, however, If you really want a "Readonly Array", you can roll your own....sort of.
If you don't need all of the bells an whistles of the Array class, you can make a pseudoArray quite
easily.

This probably should be generic, but you get the idea
This is a bare minimum

// Minimal version
public class ReadOnlyIntArray
{
private int[] data;
public ReadOnlyIntArray(int[] data)
{
this.data = data;
}

public int this[int idx] {get{return(data[idx] );}} // didn't even bounds check
public int Length {get{return(data.Length);}}
}

This works fine for Accessing elements via indexer, but you can't do foreach since in does not
support IEnumerable.

If you want foreach support you can do this

// IEnumerable, IEnumerator version
public class ReadOnlyIntArray : IEnumerable, IEnumerator
{
private int currentIdx = -1;
private int[] data;
public ReadOnlyIntArray(int[] data)
{
this.data = data;
}

public int this[int idx] {get{return(data[idx] );}} // didn't even bounds check
public int Length {get{return(data.Length);}}

public IEnumerator GetEnumerator()
{
Reset();
return (this);
}

public object Current {get {return data[currentIdx]; }} // needs bounds check

public bool MoveNext()
{
bool ret = true;
currentIdx++;
if (currentIdx == data.Length)
ret = false;
return (ret);
}

public void Reset()
{
currentIdx = -1;
}
}

These classes were quick and dirty and need a bit of work to be more robust and behave correctly,
but you get the idea.

Hope this helps
Bill

 

[steve bull]

this would limit me to one read only array per class. I have a polygon class and it will contain arrays of vertices and
arrays of edges etc. Generally I only want to iterate through the arrays. I will probably keep it simple and use a
method for access.

still seems to me that this is really poor design on MSs part.


thanks for the help,

steve

unfortunately the indexer cannot have a name so the index is on the class as a whole. which pretty
much seems like a
waste of time. MS don't appear to have spent too much time thinking about this.

This is true, however, If you really want a "Readonly Array", you can roll your own....sort of.
If you don't need all of the bells an whistles of the Array class, you can make a pseudoArray quite
easily.

This probably should be generic, but you get the idea
This is a bare minimum

// Minimal version
public class ReadOnlyIntArray
{
private int[] data;
public ReadOnlyIntArray(int[] data)
{
this.data = data;
}

public int this[int idx] {get{return(data[idx] );}} // didn't even bounds check
public int Length {get{return(data.Length);}}
}

This works fine for Accessing elements via indexer, but you can't do foreach since in does not
support IEnumerable.

If you want foreach support you can do this

// IEnumerable, IEnumerator version
public class ReadOnlyIntArray : IEnumerable, IEnumerator
{
private int currentIdx = -1;
private int[] data;
public ReadOnlyIntArray(int[] data)
{
this.data = data;
}

public int this[int idx] {get{return(data[idx] );}} // didn't even bounds check
public int Length {get{return(data.Length);}}

public IEnumerator GetEnumerator()
{
Reset();
return (this);
}

public object Current {get {return data[currentIdx]; }} // needs bounds check

public bool MoveNext()
{
bool ret = true;
currentIdx++;
if (currentIdx == data.Length)
ret = false;
return (ret);
}

public void Reset()
{
currentIdx = -1;
}
}

These classes were quick and dirty and need a bit of work to be more robust and behave correctly,
but you get the idea.

Hope this helps
Bill