views:

45

answers:

1

I'm writing a C# library to wrap a Win32 API (the waveOut... family of functions), and have reached a point where I'm unsure how to manage the interaction between different parts of my code without breaking encapsulation. So far, I have a setup like this:

public class AudioDevice
{
    ...
    private WaveOutSafeHandle hWaveOut;
    ...
}

// All native method calls happen in wrapper methods here, providing
// uniformity of access, exceptions on error MMRESULTs, etc.
static class NativeWrappers
{
    ...
    internal static WaveOutSafeHandle OpenDevice(...) { ... waveOutOpen(...) ... }
    ...
}

// Native methods live in a class of their own, and are only called
// from NativeWrappers
static class NativeMethods
{
    ...
    internal static extern MMResult waveOutOpen(...);
    ...
}

The most important point in this code is that the handle wrapped by a Device is not visible to anything outside a Device.

Now I want to add an AudioBlock class, which will wrap the native WAVEHDR structure and the audio sample data. The problem I'm encountering is that from here on out, pretty much every other native function I'm interested in (waveOut[Un]PrepareHeader, waveOutWrite) needs both a handle and a WAVEHDR. It seems that either a device will have to touch a WAVEHDR, or a block will have to have access to a handle. Either approach means that some class interacts with something it conceptually has no business knowing about.

There are, of course, several ways to get around this:

  1. Make handles and/or WAVEHDRs internal rather than private.

  2. Make AudioBlock a nested class of Device.

  3. Have a third class (I hesitate to even think the name (foo)Manager) which maintains (for example) a mapping from blocks to headers, which, given a block, a device can use to help it play samples without touching the block's internals.

There may be others; I'm hoping so :)

My objections (right or wrong) to these approaches:

  1. They might not be public in the strictest sense of the word, but using internal members seems like a copout to me. What are effectively implementation details are still visible to parts of the library which don't need them. I keep thinking "what interface do I want to present to anyone either using or modifying this code?"

  2. This almost works in my head. By regarding a block as an "implementation detail" of a device, it seems more acceptable to allow it to rely on a device's internals. Except that a block really is an independent entity; it's not tied to a single device and isn't used to help implement a device's logic.

  3. This gets the closest to the level of separation I want to maintain, but is starting to stray into overengineering territory, as I so often do :P It also introduces the artificial idea that blocks have to be centrally allocated from somewhere to keep the mapping intact.

So, does anyone have any recommendations for (re)structuring this code? Valid answers include "Your objection #X is a steaming crock," as long as you can persuade me :) ETA: For example, if you think trying to enforce this kind of thing is better done by social means (documentation, conventions) than technical ones (access modifiers, assembly boundaries), please let me know and point me to some references.

+2  A: 

They might not be public in the strictest sense of the word, but using internal members seems like a copout to me.

Personally, I'd just make the wrappers internal, and treat your whole set of classes as a single public API.

I understand the desire to avoid this - it forces you to create classes, which, for you during your development, violate the single responsibility principles.

However, from the POV of the "outside" world, anybody using your software will see each class you provide having a single, clear responsibility, and a single purpose. The API can be at least as clean as the ones you're wrapping (probably much simpler, given the managed code).

My main motivation for doing this, in this situation, is one of practicality. Yes, I agree with the guidelines you're trying to follow here - but, they're guidelines, and guidelines are something worth following provided they don't cause more harm than good. I commend you for trying to keep this as clean and elegant as possible, but unfortunately, it sounds like, in this situation, trying to make this "more elegant" is going to lead to more code, which will equate to less maintainable.

I'd stick with the shortest, simplest solution here - making the native wrappers internal, so you can get to the data structures you need in your wrapper classes. Just document what you're doing, and why.

Reed Copsey
This is good advice. The way I'd respond to the issue of violating single responsibility is that these internal classes each have the single responsibility of wrapping one API object. The public, external classes have the responsibility to use these wrappers to create the desired functionality.
Steven Sudit
Thanks very much for the answer. I suspected this was another case of trying to keep things "too" elegant. It's something I do a lot, and leads to much frustration :P Pragmatism ftw!
shambulator