views:

233

answers:

5

According to MSDN: Array usage guidelines:

Array Valued Properties

You should use collections to avoid code inefficiencies. In the following code example, each call to the myObj property creates a copy of the array. As a result, 2n+1 copies of the array will be created in the following loop.

[Visual Basic]

Dim i As Integer
For i = 0 To obj.myObj.Count - 1
   DoSomething(obj.myObj(i))
Next i

[C#]
for (int i = 0; i < obj.myObj.Count; i++)
      DoSomething(obj.myObj[i]);

Other than the change from myObj[] to ICollection myObj, what else would you recommend? Just realized that my current app is leaking memory :(

Thanks;

EDIT: Would forcing C# to pass references w/ ref (safety aside) improve performance and/or memory usage?

+5  A: 

No, it isn't leaking memory - it is just making the garbage collector work harder than it might. Actually, the MSDN article is slightly misleading: if the property created a new collection every time it was called, it would be just as bad (memory wise) as with an array. Perhaps worse, due to the usual over-sizing of most collection implementations.

If you know a method/property does work, you can always minimise the number of calls:

var arr = obj.myObj; // var since I don't know the type!
for (int i = 0; i < arr.Length; i++) {
  DoSomething(arr[i]);
}

or even easier, use foreach:

foreach(var value in obj.myObj) {
  DoSomething(value);
}

Both approaches only call the property once. The second is clearer IMO.

Other thoughts; name it a method! i.e. obj.SomeMethod() - this sets expectation that it does work, and avoids the undesirable obj.Foo != obj.Foo (which would be the case for arrays).

Finally, Eric Lippert has a good article on this subject.

Marc Gravell
You can also make the property getter smart, and cache the array that is returned between calls. Obviously you need to intelligently invalidate the cache when necessary.
Ch00k
@Ch00k - but since arrays are mutable, that isn't safe; see Eric's article in my updated reply - you'd have to use a read-only colletion for that.
Marc Gravell
+1  A: 

Whenever I have properties that are costly (like recreating a collection on call) I either document the property, stating that each call incurs a cost, or I cache the value as a private field. Property getters that are costly, should be written as methods. Generally, I try to expose collections as IEnumerable rather than arrays, forcing the consumer to use foreach (or an enumerator).

Øyvind Skaar
A: 

It will not make copies of the array unless you make it do so. However, simply passing the reference to an array privately owned by an object has some nasty side-effects. Whoever receives the reference is basically free to do whatever he likes with the array, including altering the contents in ways that cannot be controlled by its owner.

One way of preventing unauthorized meddling with the array is to return a copy of the contents. Another (slightly better) is to return a read-only collection.

Still, before doing any of these things you should ask yourself if you are about to give away too much information. In some cases (actually, quite often) it is even better to keep the array private and instead let provide methods that operate on the object owning it.

norheim.se
A: 

myobj will not create new item unless you explicitly create one. so to make better memory usage I recommend to use private collection (List or any) and expose indexer which will return the specified value from the private collection

Ahmed Said
+2  A: 

Just as a hint for those who haven't use the ReadOnlyCollection mentioned in some of the answers:

[C#]

class XY
{
  private X[] array;

  public ReadOnlyCollection<X> myObj
  {
    get
    {
      return Array.AsReadOnly(array);
    }
  }
}

Hope this might help.

rstevens