search

Kyle-Gagner / PortAudio.Net

Cross platform .NET wrappers for PortAudio
Mozilla Public License 2.0
12 stars 7 forks source link

Use SafeHandle in place of IDisposable #2

Open ForeverZer0 opened 5 years ago

ForeverZer0 commented 5 years ago

When implementing bindings for native code, the more modern and preferred way is to inherit and implement from SafeHandle class. There are many benefits, and at the very least will simplify the typical dispose pattern and eliminate the need for any destructor.

Kyle-Gagner commented 4 years ago

I will evaluate this and get back to you on it.

Kyle-Gagner commented 4 years ago

Actually, I'm not sure I understand your request. Would this change the API surface (remove IDisposable from public types) or just implementation details (use SafeHandle where possible to ensure unmanaged resources are cleaned up)?

ForeverZer0 commented 4 years ago

It wouldn't change the API at all. A Safehandle is an IDisposable, and considering that wrappers in the library don't even inherit from anything other than Object, it would simply be a matter adding the inheritance, calling the constructor, and implementing the ReleaseHandle method. You could then remove all the boiler-plate code with Dispose(), Dispose(bool), and the destructor.

The documentation on the SafeHandle class have some full examples on replacing the legacy-style dispose pattern that uses destructors, it is pretty painless to implement.

Kyle-Gagner commented 4 years ago

I see what you're saying now. To keep a clean API surface I would not inherit from SafeHandle on public types, but I might create internal implementations of SafeHandle to simplify the disposal of unmanaged resources. That keeps good separation of concerns between API and implementation. Right now, I don't even have XML docs on all public API members. I'm hoping to put some more effort into this project soon, so XML docs and other public API concerns will come first, then I will look at this issue. Thanks for the feedback.