[Mono-osx] CoreMIDI Progress

Miguel de Icaza miguel at novell.com
Mon Jan 10 16:59:07 EST 2011


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

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


More information about the Mono-osx mailing list