std :: vectorクラスの実装

Sep 14 2020

ベクトルクラスの機能を複製する私の試みについてのフィードバックをお待ちしております。特に、コピーコンストラクタとサイズ変更メソッドについて疑問があります。コピーコンストラクタはメモリリークを引き起こす可能性があると思いますが、それを改善する方法がわかりません。2つのサイズ変更方法を実装しましたが、どちらも機能しているようですが、実際にはどちらが優れていますか?

#include <iostream>
#include <type_traits>
#include <math.h>

template <typename T>
class Vector {
private:
    T* m_Data;
    size_t m_Size, m_Capacity;
public:
    Vector(size_t cap = 2)
        : m_Size(0), m_Capacity(cap) {
        m_Data = new T[cap];
    }
    
    Vector(size_t size, size_t cap)
        : m_Size(size), m_Capacity(cap) {
        m_Data = new T[cap];
    }

    Vector(const std::initializer_list<T>& il)
        : Vector(il.size(), il.size() * 2) {
        int cnt = 0;
        for (const auto& el : il)
            m_Data[cnt++] = el;
    }
   
    // copy constructor, makes deep copy
    Vector(const Vector& v)
        : m_Size(v.size()), m_Capacity(v.capacity()) {
        m_Data = new T[m_Capacity];
        for (size_t i = 0; i < m_Size; i++) {
            m_Data[i] = v[i];
        }
    }
    
    ~Vector() {
        delete[] m_Data;
    }

//    void resize(size_t newCapacity) {
//        T* newData = new T[newCapacity];
//        m_Size = std::min(m_Size, newCapacity);
//
//        for (size_t i = 0; i < m_Size; i++)
//            newData[i] = std::move(m_Data[i]);
//
//        delete[] m_Data;
//        m_Data = newData;
//        m_Capacity = newCapacity;
//    }
    
    void resize(size_t newCapacity) {
        char* newData = new char[sizeof(T) * newCapacity];
        m_Size = std::min(m_Size, newCapacity);
        
        T* dst = reinterpret_cast<T*>(newData);
        for (size_t i = 0; i < m_Size; i++)
            new (dst + i) T(m_Data[i]);
        
        delete[] m_Data;
        m_Data = reinterpret_cast<T*>(newData);
        m_Capacity = newCapacity;
    }
    

        
    void push_back(const T& n) {
        if (m_Capacity <= m_Size)
            resize(m_Capacity * 2);
    
        m_Data[m_Size++] = n;
    }
    
    void push_back(const T&& n) {
        if (m_Capacity <= m_Size)
            resize(m_Capacity * 2);
    
        m_Data[m_Size++] = std::move(n);
    }
    
    void pop_back() {
        if (m_Size > 0)
            m_Data[--m_Size].~T();
    }

    void clear() {
        for (size_t i = 0; i < m_Size; i++)
            m_Data[i].~T();
        m_Size = 0;
    }
    
    size_t size() const {
        return m_Size;
    }
    
    size_t capacity() const {
        return m_Capacity;
    }
    
    bool empty() const {
        return m_Size == 0;
    }
    
    const T& operator[](size_t index) const {
        if (index >= m_Size)
            throw "Index out of bounds";
        
        return m_Data[index];
    }
    
    T& operator[](size_t index) {
        if (index >= m_Size)
            throw "Index out of bounds";
        
        return m_Data[index];
    }
    
    Vector<T> operator+(const Vector& other) {
        if (m_Size != other.size())
            throw "Vectors are of different size";
        
        Vector<T> v(m_Size);
        for (size_t i = 0; i < m_Size; i++) {
            v.push_back(m_Data[i] + other[i]);
        }
        
        return v;
    }
    
};

template <
typename T,
typename = typename std::enable_if<std::is_arithmetic<T>::value, T>::type>
int norm(const Vector<T> v) {
    int nrm = 0;
    size_t n = v.size();
    
    for (int i = 0; i < n; i++) {
        nrm += v[i]*v[i];
    }
    
    return sqrt(nrm);
}

template <typename T>
std::ostream& operator<<(std::ostream& s, const Vector<T>& v) {
    s << "[";
    size_t n = v.size();
    for (size_t i = 0; i < n; i++) {
        s << v[i] << (i < n - 1 ? ", " : "");
    }
    s << "]";
    return s;
}

int main(int argc, const char * argv[]) {
    Vector<int>* a = new Vector<int> {1, 2, 3, 4, 5};
    Vector<int> b = {5, 4, 3, 2, 6};
    Vector<int> c = b;

    return 0;
}

ありがとうございました。

回答

7 G.Sliepen Sep 14 2020 at 17:34

コピーコンストラクタと resize()

コピーコンストラクター(および他のコンストラクター)では、を使用してメモリを割り当てますnew T[...]が、でresize()メモリを割り当ててnew char[sizeof(T) * ...]から、placementnewを使用して古い要素をコピーします。前者は安全ですが、予想よりも多くのコンストラクタを呼び出す可能性があります。後者には、適切に初期化されなかった未使用の容量が存在する可能性があるという問題がありますが、deleteそれを行うと、すべての予約済み要素でデストラクタが呼び出されます。

安全Tを確保し、予約済み要素のコンストラクターを呼び出さないようにするには、次のことを一貫して行います。

  • char *m_Data割り当てられたメモリを追跡するために使用します(として保持することもできますがT *m_Data、delete[] m_Data直接呼び出さないように注意する必要があります)
  • newベクトルに実際の要素を追加するときは、常に配置を使用してください
  • ベクトルから実際の要素を削除するときは、常に「配置delete」を使用してください

また、理想的std::moveにはresize()、の間に要素を作成する必要がありますが、特にTのmoveコンストラクターが例外をスローする可能性がある場合は注意が必要です。

からの発散 std::vector

コメントですでに説明したように、ベクタークラスはとは少し異なりstd::vectorます。これは、割り当ての要件によるものです。クラスの割り当て以外にも、を使用できないがstd::vector、自分で実装する必要がある実際のシナリオがあります。その場合std::vector、自分のクラスがドロップインの代替品であることを確認するために、インターフェイスを可能な限り同じに保ちたいと思います。驚くことはありません。

コンストラクター予約スペースと割り当て要素

size_t引数を取るコンストラクターは、引数を使用してスペースを予約しますが、ベクトルに要素を追加しません。ただし、からの対応するコンストラクターstd::vectorは、引数を使用して、デフォルトで初期化される実際の要素を割り当てます。また、あなたのクラスで:

Vector<int> v(4, 2);

これにより、初期化されていない2つの要素にスペースのみが割り当てられ、m_Size4に設定されてサイズが互いに不整合になり、後続の呼び出しでoperator[]()エラーをスローせずに範囲外を読み取ることができます。これを以下と比較してください:

std::vector<int> v(4, 2);

これにより、すべて値に初期化される4つの要素のベクトルが割り当てられます2。したがって、まったく異なる動作です。

size_tカウンターに使用

初期化子リストを受け取るコンストラクターでは、を使用しますint cntが、int十分な大きさではない可能性があります。size_tサイズ、カウント、インデックスには一貫して使用してください。

メンバー初期化子リストでメモリーを割り当てることができます

メンバー初期化子リストには、メモリの割り当てなどの副作用を含む、より複雑な式を含めることができることに注意してください。だからあなたは書くことができます:

Vector(size_t size, size_t cap)
    : m_Data(new T[cap]), m_Size(size), m_Capacity(cap) {}

この場合は実際には問題ではありませんが、場合によっては利点があるため、これを行うことをお勧めします。

適切な例外タイプを使用してスローする

throw文字列をランダムにしないでください。ただし、例外には適切なタイプを使用してください。標準ライブラリを使用する場合は、次のように適切なタイプを選択します。

if (index >= m_Size)
    throw std::out_of_range("Index out of bounds");

標準ライブラリを使用できない場合は、少なくとも独自の例外タイプを定義して、呼び出し元が特定のキャッチブロックを使用できるようにします。たとえば、次のことを実行したいとします。

try {
    Vector<int> v(100000); // might throw std::bad_alloc if `new` fails
    Vector<int> w(10000);
    v[100000] = 10;   // out of range error
    v += w;           // vectors of different size
}
catch (std::bad_alloc &e) {
    // out of memory
}
catch (std::out_of_bounds &e) {
    // handle index out of bounds
}
catch (std::invalid_argument &e) {
    // handle operator+[] with an argument of the wrong size
}

文字列をスローするだけの場合、catchブロックは1つしか持てません。ブロックは、文字列を解析して何が起こっているのかを把握する必要があります。

境界チェックインを行わないことを検討してください operator[]()

標準ライブラリはoperator[]()、パフォーマンスに大きな影響を与えるため、を使用するときに境界チェックを行いません。at()境界チェックを行う別の関数、があります。

Vector<T>中に書く必要はありませんVector

クラス定義内では、書く必要はありません。書くVector<T>だけVectorです。

イテレータがありません

クラスはイテレータを実装していないため、次のようなものを書くことはできません。

Vector<int> v(10);
...
for (auto el: v) {
    std::cout << el << "\n";
}

クラスにイテレータを実装してみるのは良い練習です。