pointer is less times cast by storing it correct, copy-swap idiom

Main.cpp

@@ -46,7 +46,7 @@ int A::instances = 0;
 
 template<typename T>
 class Vector {
-    char* data;
+    T* data;
     int capacity;
     int elements;
 
@@ -55,82 +55,63 @@ public:
     }
 
     ~Vector() {
-        destroy();
+        // placement new needs explicit destructor calling
+        for(int i = 0; i < elements; i++) {
+            data[i].~T();
+        }
+        delete[] reinterpret_cast<char*>(data);
     }
 
     // allocate new storage for copies
-    Vector(const Vector& other) : data(allocate(other.capacity)), capacity(other.capacity), elements(other.elements) {
+    Vector(const Vector& other)
+        : data(allocate(other.capacity)), capacity(other.capacity),
+          elements(other.elements) {
         // copy data into new storage
-        other.copyTo(data);
-    }
-
-    Vector& operator=(const Vector& other) {
-        if(this != &other) {
-            // clear current resources
-            destroy();
-            // allocate new storage for copies
-            data = allocate(other.capacity);
-            capacity = other.capacity;
-            elements = other.elements;
-            // copy data into new storage
-            other.copyTo(data);
+        for(int i = 0; i < elements; i++) {
+            new(data + i) T(other.data[i]);
         }
-        return *this;
     }
 
-    // acquire the given other vector
-    Vector(Vector&& other) : data(other.data), capacity(other.capacity), elements(other.elements) {
-        // remove everything from the other vector
-        other.reset();
+    // this handles copy and move assigment
+    // copy: replace current resources with other made by the copy constructor
+    // move: replace current resources with other made by the move constructor
+    // other gets the old data and is destroyed by the destructor after going
+    // out of scope
+    Vector& operator=(Vector other) {
+        swap(*this, other);
+        return *this;
     }
 
-    Vector& operator=(Vector&& other) {
-        if(this != &other) {
-            // clear current resources
-            destroy();
-            // acquire the given other vector
-            data = other.data;
-            capacity = other.capacity;
-            elements = other.elements;
-            // remove everything from the other vector
-            other.reset();
-        }
-        return *this;
+    // construct a default vector so the other vector gets valid values from the
+    // swap
+    Vector(Vector&& other) : Vector() {
+        swap(*this, other);
     }
 
     void reserve(int size) {
         if(size <= capacity) {
             return;
         }
-        capacity = size;
-        char* newData = allocate(capacity);
-        // nothing todo without any old data
-        if(data == nullptr) {
-            data = newData;
-            return;
+        Vector v;
+        v.capacity = size;
+        v.data = allocate(size);
+        for(int i = 0; i < elements; i++) {
+            v.push_back(std::move(data[i]));
         }
-        // there are elements which need to be moved
-        moveTo(newData);
-        destroy();
-        data = newData;
+        swap(*this, v);
     }
 
     void resize(int size, const T& t) {
+        // elements will be equal to size after this call but not the capacity
         if(size > elements) {
-            // move existing object to new storage
-            char* newData = allocate(size);
-            moveTo(newData);
-            destroy();
-            data = newData;
-            capacity = size;
-            // fill the rest with the given object
+            // fill until the given size is reached
             for(int i = elements; i < size; i++) {
                 push_back(t);
             }
         } else if(size < elements) {
             // remove objects until the size matches
             for(int i = size; i < elements; i++) {
-                (*pointer(data, i)).~T();
+                data[i].~T();
             }
             elements = size;
         }
@@ -142,21 +123,21 @@ public:
 
     void push_back(const T& t) {
         ensureCapacity();
-        new(pointer(data, elements++)) T(t);
+        new(data + elements++) T(t);
     }
 
     void push_back(T&& t) {
         ensureCapacity();
         // && would be lost without std::move
-        new(pointer(data, elements++)) T(std::move(t));
+        new(data + elements++) T(std::move(t));
     }
 
     T& operator[](int index) {
-        return *pointer(data, index);
+        return data[index];
     }
 
     const T& operator[](int index) const {
-        return *pointer(data, index);
+        return data[index];
     }
 
     T& at(int index) {
@@ -176,19 +157,19 @@ public:
     }
 
     T* begin() {
-        return pointer(data, 0);
+        return data;
     }
 
     T* end() {
-        return pointer(data, elements);
+        return data + elements;
     }
 
     const T* begin() const {
-        return pointer(data, 0);
+        return data;
     }
 
     const T* end() const {
-        return pointer(data, elements);
+        return data + elements;
     }
 
     void erase(T* from, T* to) {
@@ -221,17 +202,23 @@ public:
         return begin();
     }
 
-private:
-    static char* allocate(int length) {
-        return new char[sizeof(T) * length];
+    void erase_by_swap(int index) {
+        elements--;
+        if(index != elements) {
+            data[index] = std::move(data[elements]);
+        }
+        data[elements].~T();
     }
 
-    static T* pointer(char* data, int index) {
-        return reinterpret_cast<T*>(data) + index;
+private:
+    static T* allocate(int length) {
+        return reinterpret_cast<T*>(new char[sizeof(T) * length]);
     }
 
-    static const T* pointer(const char* data, int index) {
-        return reinterpret_cast<const T*>(data) + index;
+    static void swap(Vector& a, Vector& b) {
+        std::swap(a.data, b.data);
+        std::swap(a.capacity, b.capacity);
+        std::swap(a.elements, b.elements);
     }
 
     void ensureCapacity() {
@@ -240,36 +227,6 @@ private:
             reserve(capacity == 0 ? 1 : capacity * 2);
         }
     }
-
-    void copyTo(char* newData) const {
-        for(int i = 0; i < elements; i++) {
-            new(pointer(newData, i)) T(*pointer(data, i));
-        }
-    }
-
-    void moveTo(char* newData) {
-        for(int i = 0; i < elements; i++) {
-            new(pointer(newData, i)) T(std::move(*pointer(data, i)));
-        }
-    }
-
-    void destroy() {
-        callDestructors();
-        delete[] data;
-    }
-
-    void reset() {
-        data = nullptr;
-        capacity = 0;
-        elements = 0;
-    }
-
-    void callDestructors() {
-        // placement new needs explicit destructor calling
-        for(int i = 0; i < elements; i++) {
-            (*pointer(data, i)).~T();
-        }
-    }
 };
 
 void printError(int number) {
@@ -286,7 +243,8 @@ void test() {
         }
         const V& cv = v;
         for(int i = 0; i < elements; i++) {
-            if(v[i].a != i || cv[i].a != i || v.at(i).a != i || cv.at(i).a != i) {
+            if(v[i].a != i || cv[i].a != i || v.at(i).a != i ||
+               cv.at(i).a != i) {
                 printError(1);
             }
         }
@@ -310,7 +268,8 @@ void test() {
         V v6;
         v6 = std::move(v3);
 
-        if(v1.size() != 0 || v2.size() != 0 || v3.size() != 0 || v4.size() != 1 || v5.size() != 1 || v6.size() != 1) {
+        if(v1.size() != 0 || v2.size() != 0 || v3.size() != 0 ||
+           v4.size() != 1 || v5.size() != 1 || v6.size() != 1) {
             printError(4);
         }
     }
@@ -345,8 +304,33 @@ void test() {
     }
 }
 
+void test() {
+    {
+        Vector<A> v;
+        v.push_back(1);
+        v.push_back(2);
+        v.push_back(3);
+        v.erase_by_swap(1);
+        if(v.size() != 2 || v[0].a != 1 || v[1].a != 3) {
+            printError(9);
+        }
+        v.erase_by_swap(1);
+        if(v.size() != 1 || v[0].a != 1) {
+            printError(10);
+        }
+        v.erase_by_swap(0);
+        if(v.size() != 0) {
+            printError(11);
+        }
+    }
+    if(A::instances != 0) {
+        std::cout << "object counter is not 0: " << A::instances << "\n";
+    }
+}
+
 int main() {
     test<Vector<A>>();
+    test();
     std::cout << "--------------------------\n";
     test<std::vector<A>>();
 }