[Mono-osx] CoreMIDI Progress

[Peter Allen Webb]

Thanks Miguel,

Your suggestions make sense to me, except one. The MIDIObjectRef values
returned by the various functions in the CoreMIDI API are not pointers but
rather unsigned 32-bit integers used as unique identifiers for the various
"midi objects". Given that fact, I don't think it makes sense to use
INativeObject, unless I am confused about something.

-- Pete

On Mon, Jan 10, 2011 at 4:59 PM, Miguel de Icaza <miguel at novell.com> wrote:

> Hello Peter,
>     I have started to review the code in CoreMIDI, and here are some
> of my suggestions:
>
> Defs.cs:
> ======
> * Make sure that all public field values start with an uppercase letter,
>  to match the Framework Design Guidelines (example: MIDIPacket
>  field names)
>
> * The word "MIDI" in the class names needs to be renamed Midi,
>  per the Framework Design Guidelines.
>
> * Drop the kMIDIXXX prefix from enumerations, these are necessary
>  in C, because they act as a mechanism to avoid name clashes, but
>  in C# we are using namespaces, and the enums themselves, so
>  what today passes as MIDIObjectType.kMIDIObjectType_Other should
>  be renamed to just Other, so that you reference it as
>  MIDIObjectType.Other
>
> * The above applies to all of the enumerations
>
> * The same applies to the public static IntPtr fields that are used as
>   keys
>
> MIDIObject.cs
> ==========
>
> Although this is an abstract class, it would be best if it would follow
> the same patterns that are used in CoreGraphics for object management
> and if it implemented the INativeObject interface as this will allow
> passing Midi objects to Objective-C functions if they ever exist (the
> marshaller and runtime depend on this).
>
> This means that the ObjectRef property should be called handle, and
> it should expose a public property Handle.
>
> MIDIObject Derived classes
> ====================
>
> Since they will be implementing the INativeObject interface, they should
> also follow the convention for the marshaller to have two constructors
> the Foo (IntPtr handle) and Foo (IntPtr handle, bool owns) to do the
> memory management.   And add the proper codepath to take the
> reference when we do not own the object.
>
> The Dispose method needs to follow the same pattern as the objects
> in CoreGraphics, as it is important to expose a virtual method that
> subclasses can override.   Disposing of the object also needs to be
> guarded.
>
> In general this is a good resource on how to implement the IDisposable
> interface:
>
> http://msdn.microsoft.com/en-us/library/ms244737(v=vs.80).aspx<http://msdn.microsoft.com/en-us/library/ms244737%28v=vs.80%29.aspx>
>
> Since MIDIEndpoint could return 2 separate instances for the same
> handle (due to the GetSourceByIndex), it should really implement
> the Equals, operator ==, operator != (and by extension GetHashCode)
> methods.
>
> As a matter of consistency, GetSourceByIndex should be renamed
> FromSourceIndex, and the same pattern applies to the destination one.
>
> MIDIClient
> =======
>
> You can use the CFString in the constructor with the using clause,
> to dispose it as soon as you are done with it.
>
> When throwing exception from a constructor, we typically also add
> a throw-less variant as a static method, that returns null on error,
> so a FromName () would do in this case.
>
> Since you create MidiInputPorts here and again you could end up
> with two objects created for the same underlying handle, this
> requires the operator ==, operator != and Equals to be implemented
> (and again, by extension, GetHashCode)
>
> Miguel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-osx/attachments/20110111/4132923a/attachment.html