Is it bad practice to put a lot of code in the get function of a property which wraps PInvoke stuff?

Question

A confusing title I know. Let me explain.

I have to marshal an array of structs, which then get converted to an array of classes (legacy compatibility). For instance

public class InnerClass {}

public class OuterClass { private InnerClass[] innerClasses; }


public struct InnerStruct {
   // Data
}

private static buildInnerClass( InnerStruct i );

public struct OuterStruct {
   private int _numInnerStructs;
   private IntPtr _innerStructs;
   // Other members

   public InnerClass[] InnerClasses {
      get {
             InnerClass[] ret = new InnerClass[_numInnerStructs];

             var structSize = Marshal.SizeOf(typeof(InnerStruct));
             var ptr = _innerStructs;

             for (int i = 0; i < _numInnerStructs; i++)
             {
                InnerStruct innerStruct = (InnerStruct)Marshal.PtrToStructure(
                                          ptr, typeof(InnerStruct));
                ret[i] = buildInnerClass(innerStruct);
                ptr = (IntPtr)((int)ptr + structSize);
             }
             return ret;
      }
   }

}

Answer 1

It is not any worse to put that code in a PInvoke struct than it is to put it in a normal struct. For the purpose of PInvoke, only the fields of a struct are considered and hence your property code won't come into play. It will only be relevant when your code accesses that particular property.

Answer 2

The getter of a property should be as minimal as possible. In your case, it's probably better to use a regular method. Especially because returning an array from a property is also a bad idea.

Answer 3

Viewed in a vacuum, there's nothing intrinsically wrong with this practice. However, you should be careful in that...

1. Property getters should--with few (if any) exceptions--not be "expensive" (ie, shouldn't consume many CPU cycles or resources to execute)
2. Property getters should NEVER cause side effects. For example, if your PInvoke code obtains a new handle of some kind then it should be a function, not a getter.

In general, keep in mind that properties should be written in such a way that the consumer shouldn't have much, if any, reason to cache the value versus calling the property again. If a responsible developer could follow that practice with your property, then you're on the right track.