PositionableAudioSource and SRP

#1

It seems that PositionableAudioSource violates the Single Responsibility Principle, on account of isLooping() and setLooping(). One could argue getTotalLength() doesn’t belong there either. It’s easy to imagine a simple LoopingAudioSource that wraps a PositionableAudioSource and provides the looping functionality (and constructs with a “total length” parameter.

I bring this up because I have a whole hierarchy of PositionableAudioSource-derived classes that I would like to publish in VFLib, however functions like isLooping() and getTotalLength() have no real meaning for these classes in the context they are designed for. I have to resort to things like this:

bool BufferSource::isLooping() const
{
  // unsupported
  jassertfalse;
  return false;
}

void BufferSource::setLooping (bool shouldLoop)
{
  // unsupported
  jassertfalse;
}

This doesn’t seem particularly clean, and I have to ask, are these here just for legacy / convenience purposes or am I missing something?

#2

It’s a fair comment, and a lot of those audiosource classes are very old. If I was designing them now, I would probably do things a bit differently.

#3

I figured out a way around it…I wrote a Facade around AudioSource to exclude the undesired functions and then wrote a couple of adapters to make it play nice with JUCE APIs that expect a PositionableAudioSource:

/**
  Facade for @ref PositionableAudioSource.

  This Facade provides an alternative interface to @ref PositionableAudioSource
  with the following features:

  - No thread safety; the caller is responsible for all synchronization.

  - The looping and total length features have been removed.

  @ingroup vf_audio
*/
class SeekingAudioSource : public AudioSource
{
public:
  /**
    Move the read position.

    Calling this indicates that the next call to @ref AudioSource::getNextAudioBlock()
    should return samples from this position.
  */
  virtual void setNextReadPosition (int64 newPosition) = 0;

  /**
    Returns the position from which the next block will be returned.

    @see setNextReadPosition
  */
  virtual int64 getNextReadPosition() const = 0;

public:
  //============================================================================
  /**
    Adapter to appear as a @ref PositionableAudioSource.

    @ingroup vf_audio
  */
  class PositionableAdapter : public PositionableAudioSource, Uncopyable
  {
  public:
    PositionableAdapter (SeekingAudioSource* source,
                         bool takeOwnership,
                         int64 totalLength);

    void setNextReadPosition (int64 newPosition);

    int64 getNextReadPosition() const;

    int64 getTotalLength() const;

    bool isLooping() const;

    void setLooping (bool shouldLoop);

    void prepareToPlay (int samplesPerBlockExpected,
                        double sampleRate);

    void releaseResources();

    void getNextAudioBlock (const AudioSourceChannelInfo& bufferToFill);

  private:
    OptionalScopedPointer <SeekingAudioSource> m_source;
    int64 const m_totalLength;
    bool m_shouldLoop;
  };

public:
  class Adapter;
};

//==============================================================================
/**
  Adapter to wrap a @ref PositionableAudioSource.

  @ingroup vf_audio
*/
class SeekingAudioSource::Adapter : public SeekingAudioSource
{
public:
  Adapter (PositionableAudioSource* source, bool takeOwnership);

  void setNextReadPosition (int64 newPosition);

  int64 getNextReadPosition() const;

  void prepareToPlay (int samplesPerBlockExpected,
                      double sampleRate);

  void releaseResources();

  void getNextAudioBlock (AudioSourceChannelInfo const& bufferToFill);

private:
  OptionalScopedPointer <PositionableAudioSource> m_source;
};

I think C++11 offers a better way of doing this using the ‘delete’ and ‘final’ keywords on an override (but I don’t have C++11 on all supported targets).