用于音频处理的环形缓冲器(跟踪)

Question

这个问题是对前一篇文章中关于用于音频处理的Ringbuffer实现的评论的后续。

用于音频处理的环形缓冲器

template <typename T>
class Array {
public:
    Array();

    Array(std::size_t const& cap);

    Array(Array<T> const& other);

    Array<T>& operator=(Array<T> other);

    virtual ~Array();

    void swap(Array<T>& first, Array<T>& second);

    std::size_t const& Capacity()const;

    void Resize(std::size_t cap);

    T& operator[](std::size_t const& index);

    T const& operator[](std::size_t const& index)const;

protected:
    std::size_t capacity;
    T* data;
};

template<typename T>
Array<T>::Array():capacity(0),data(nullptr){}

template<typename T>
Array<T>::Array(std::size_t const& cap):capacity(cap),data(capacity ? new T[capacity]() : 0){}

template<typename T>
Array<T>::Array(Array<T> const &other):capacity(other.capacity),data(capacity ? new T[capacity]() : 0){
    std::copy(other.data, other.data + capacity, data);
}

template<typename T>
Array<T>& Array<T>::operator=(Array<T> other){//implicit copy
    if (this != &other) {
        swap(*this, other);//and swap 
    }
    return *this;
}

template<typename T>
Array<T>::~Array(){
    delete [] data;
    capacity=0;
}

template<typename T>
void Array<T>::swap(Array<T>& first, Array<T>& second){
    std::swap(first.capacity, second.capacity);
    std::swap(first.data, second.data);
}

template<typename T>
std::size_t const& Array<T>::Capacity()const{
    return capacity;
}
template<typename T>
void Array<T>::Resize(std::size_t cap){
    Array<T> temp(cap);
    swap(*this, temp);
}

template<typename T>
T& Array<T>::operator[](std::size_t const& index){
    return this->data[index];
}

template<typename T>
T const& Array<T>::operator[](std::size_t const& index)const{
    return this->data[index];
}

将代码分成两部分，便于阅读。

#define MAKE_POW_2(number)( pow(2, ceil(log(number)/log(2))))//round up to the nearest power of two

template<typename T>
class Ringbuffer{
public:
    Ringbuffer();

    Ringbuffer(std::size_t cap);

    ~Ringbuffer();

    Ringbuffer(Ringbuffer<T> const& other);

    Ringbuffer<T>& operator=(Ringbuffer<T> other);

    bool Read(T& location);

    bool Write(T const& value);

    inline bool IsFull()const;

    inline bool IsEmpty()const;

    void swap(Ringbuffer<T>& first, Ringbuffer<T>& second);

    std::size_t const& Count()const;

    std::size_t const& Capacity()const;

    void Resize(size_t cap);

    void Initialize();
protected:
    inline void next_index(size_t& current)const;//increment index variable with wraparound

    Array<T> buffer;
    std::size_t read;
    std::size_t write;
    std::size_t count;
    std::size_t mask;
};

template<typename T>
Ringbuffer<T>::Ringbuffer():buffer(),read(0),write(0),count(0),mask(0){}

template<typename T>
Ringbuffer<T>::Ringbuffer(std::size_t cap):buffer(MAKE_POW_2(cap)),read(0),write(0),count(0){
    mask = buffer.Capacity()-1;
}

template<typename T>
Ringbuffer<T>::~Ringbuffer(){
    read=0;
    write=0;
    count=0;
    mask=0;
}

template<typename T>
Ringbuffer<T>::Ringbuffer(Ringbuffer<T> const& other):buffer(other.buffer),read(other.read),write(other.write),count(other.count){}

template<typename T>
Ringbuffer<T>& Ringbuffer<T>::operator=(Ringbuffer<T> other){
    if (this != &other) {
        swap(*this,other);
    }
    return *this;
}

template<typename T>
bool Ringbuffer<T>::Read(T& location){
    if (!IsEmpty()) {
        location = buffer[read];
        next_index(read);
        --count;
        return true;
    }
    return false;
}

template<typename T>
bool Ringbuffer<T>::Write(T const& value){
    if (!IsFull()) {
        buffer[write]=value;
        next_index(write);
        ++count;
        return true;
    }
    return false;
}

template<typename T>
inline bool Ringbuffer<T>::IsFull()const{
    return (count == buffer.Capacity());
}

template<typename T>
inline bool Ringbuffer<T>::IsEmpty()const{
    return (count==0);
}

template<typename T>
void Ringbuffer<T>::swap(Ringbuffer<T>& first, Ringbuffer<T>& second){
    std::swap(first.buffer,second.buffer);
    std::swap(first.read,second.read);
    std::swap(first.write,second.write);
    std::swap(first.mask, second.mask);
}

template<typename T>
std::size_t const& Ringbuffer<T>::Count()const{
    return count;//number of elements currently being stored
}

template<typename T>
std::size_t const& Ringbuffer<T>::Capacity()const{
    return buffer.Capacity();
}

template<typename T>
void Ringbuffer<T>::Resize(size_t cap){
    buffer.Resize();
    Initialize();
}

template<typename T>
void Ringbuffer<T>::Initialize(){
    read=0;
    write=0;
    count=0;
    mask=0;
    for (int i=0; i<buffer.Capacity(); ++i) {
        buffer[i] = T();
    }
}

template<typename T>
inline void Ringbuffer<T>::next_index(size_t& current)const{
    current = (current+1) & mask;
    //bitwise modulo increment (only works on pow 2 size arrays which is why we force pow 2 size in constructor)
}

新版本的代码假定为单个线程环境，并已作为模板编写，以提高通用性。

Flush函数被重新命名为Initialize，以使其目的更加清晰。

Full和Empty函数被重新命名为IsFull和IsEmpty，以使它们的用途更加清晰。

和往常一样，我希望在代码中解决任何可能的问题。

Answer 1

警告已经有一个std：：

数组

您可能需要小心，因为已经有一个标准数组。你不想让你的用户感到困惑。我不认为这会是个问题，但你应该记住。

不要重写容器类.

数组类基本上是对std::vector的简化。但它并没有所有的特点，使向量的效率。为什么不使用标准容器呢？如果不想公开整个向量接口，只需包装它并公开实际需要的方法。

你的风格有点狭窄的

template<typename T>
Array<T>::Array():capacity(0),data(nullptr){}

当然可以了。但是，一点空白不会杀死你，它将使人类更容易阅读。至少在逗号和冒号后面放一个空格。

就我个人而言，我会用上几句话：

template<typename T>
Array<T>::Array()
    : capacity(0)
    , data(nullptr)
{}

偏好nullptr而不是零

template<typename T>
Array<T>::Array(std::size_t const& cap)
    : capacity(cap)
    , data(capacity ? new T[capacity]() : 0)
{}                                       ^^^^^^   use nullptr here

使用nullptr将更好地表达您的意图。

昂贵拷贝

template<typename T>
Array<T>::Array(Array<T> const &other):capacity(other.capacity),data(capacity ? new T[capacity]() : 0){
    std::copy(other.data, other.data + capacity, data);
}

这里的问题是，如果T的建造和复制都是昂贵的，那么你要支付两倍的费用。没有简单的解决方案，因为对类的更改会产生连锁反应。

 new T[capacity];   // Create `capacity` objects and calls the constructor.
                    // for T on each object.

 // Now you loop over the data and 
 // and call the assignment operator T on each member.
 std::copy(other.data, other.data + capacity, data);

只要做一点工作，就可以将内存的分配与构造分开。因此，只对每个成员调用复制构造函数(从而减少了总体负载)。但这比我现在想做的要多一点。

或者，您可以使用std::vector<T>来解决这个问题。

复制和交换

当使用副本和交换成语时，不需要测试自我分配。

template<typename T>
Array<T>& Array<T>::operator=(Array<T> other){//implicit copy
    if (this != &other) {
        swap(*this, other);//and swap 
    }
    return *this;
}

注意，另一个是通过值传递的。因此，它是它自己的价值。other不可能成为this (即使发生了自分配)，因为我们已经复制了原始对象。

template<typename T>
Array<T>& Array<T>::operator=(Array<T> other)
{
    swap(*this, other);
    return *this;
}

交换通常采用一个参数

您已经将交换function与交换method混为一谈。

template<typename T>
void Array<T>::swap(Array<T>& first, Array<T>& second){
    std::swap(first.capacity, second.capacity);
    std::swap(first.data, second.data);
}

通常，您会看到这样定义的交换。

class Array
{
    public:
        void swap(Array& other) noexcept   // Important to use noexcept
        {
             // The one place were using is good.
             // Get used to doing it this way.
             // Even when not strictly required. Because if you change the
             // types of your members then this will continue to work.
             //
             // By having using here.
             // The compiler will look up a specific swap by using
             // argument dependent look-up to find the correct swap in the
             // correct namespace. If one does not exist then it can use
             // the generic one defined in the standard namespace.
             using std::swap;
             swap(capacity,   other.capacity);
             swap(data,       other.data);
        }
}
void swap(Array& lhs, Array& rhs) {
    lhs.swap(rhs);
}

注意，您还应该标记您的交换操作noexcept。

不要在析构函数

中做繁忙的工作

template<typename T>
Array<T>::~Array(){
    delete [] data;
    capacity=0;           //  This line is not needed.
                          //  After this methdo finishes the object no longer exists.
                          //  So there is no concept of a variable called capacity.
}

不要使用这个

return this->data[index];

这表明您的变量命名工作做得很糟糕，您需要显式地区分成员和本地范围变量。这不是事实，所以简单地返回你的意思。

return data[index]

注：

我上面说的大部分内容也适用于RingBuffer。我不重复了。所以下一节只剩下其他部分了。

不需要使用宏

#define MAKE_POW_2(number)( pow(2, ceil(log(number)/log(2))))

你可以用函数获得同样的表达能力。此外，函数是语言的一部分，因此需要更严格地检查其正确性。

template<typename T>
inline auto makePow2(T number)
{
    return pow(2, ceil(log(number)/log(2))));
}

不创建不需要的析构函数：

template<typename T>
Ringbuffer<T>::~Ringbuffer(){
    read=0;
    write=0;
    count=0;
    mask=0;
}

环形缓冲区没有资源管理。所以移除它的破坏者。反正也没有用。

不使用内联不需要

inline bool IsFull()const;

inline bool IsEmpty()const;

在这里没有帮助，只是把我需要阅读的东西弄乱了，才能看到界面。如果编译器需要内联，它会通过抱怨让您知道。

不要创建无用的公共函数.

inline bool IsFull()const;
inline bool IsEmpty()const;

对于应用程序来说，了解这一点有用吗？它已经被读/写方法检查了。我不知道这是你需要问的问题。应用程序能用这些信息做任何有用的事情吗？

如果没有，那么将它们排除在公共接口之外。一旦使用了公共接口，添加功能要比从公共接口中删除函数容易得多。

这是一个你绝对不应该在公共界面。

void Initialize();

构造函数进行初始化。它可以推迟到初始化方法，但不应该公开。但也许你是说重置？

不要重做工作

void Ringbuffer<T>::Initialize(){
    ...
    for (int i=0; i<buffer.Capacity(); ++i) {
        buffer[i] = T();
    }

缓冲区是一个数组。数组已经初始化了所有这些成员。你不需要重新初始化它们