4

for example, I try to write my own vector, so I just write its assign function like this 

template <typename T>
void Vector<T> :: assign(T *start, T *end)
{
    if (end - start > _capacity)
    {
        resize(end - start);
    }
    _size = end - start;
    delete []ptr;
    ptr = new T[_capacity];
    memcpy(ptr, start, end - start);
}

I have new the pointer ptr before, but I can copy all I what between the pointer start and end

why? Thankyou very much.

templatetypedef
  • 362,284
  • 104
  • 897
  • 1,065
lpy
  • 587
  • 1
  • 6
  • 20

2 Answers2

4

First problem is that this only works for simple types (read POD).
Anything with a constructor/destructor needs to have them called.

Second this is not exception safe.
It does not even provide the basic guarantee let a lone the strong guarantee.

You need to do all exception unsafe work before modifying the object. This means the new must be done before you modify the object (and definitely before the free). Otherwise you could potentially throw leaving the object in an invalid state (that might not seem bad, but what if you catch the exception and continue you now have an object that contains a pointer to released memory).

So even if you use std::copy() you still have done the wrong thing.
Personally I think the suggestion of std::copy() is a red herring. It will copy the data correctly but you are still writing your method badly. You need to use a twist on the copy and swap idium.

template <typename T>
void Vector<T> :: assign(T *start, T *end)
{
    Vector<T> tmp(start, end);  // construct a temp object that allocates the memory.



    swap(tmp);                  // Swap the current object and the tmp objects data.
                                // When the tmp object goes out of scope it will delete
                                // what was the current objects data

}
Martin York
  • 257,169
  • 86
  • 333
  • 562
3

It is perfectly fine to reuse the pointer in this way, but it is not safe to use memcpy here because you don't know what type T is. If T is an object type like string or vector, this causes undefined behavior.

To fix this, change the line to 

std::copy(start, end, ptr);

Which is the safe, C++ way of doing this.

Hope this helps!

templatetypedef
  • 362,284
  • 104
  • 897
  • 1,065
  • Thankyou, and I just found that the right way to use it is `std::copy(start, end, ptr);` – lpy Jun 08 '12 at 16:52
  • This is not a good idea. As it still leaves the `assign()` method providing no exception guarantee and thus it is a broken answer. – Martin York Jun 08 '12 at 17:03
  • The order of args to `std::copy` is incorrect. It should be `std::copy(start, end, ptr);`, not `std::copy(ptr, start, end);`. In C++11, you can use `std::copy_n(start, _size, ptr)`. – Nawaz Jun 08 '12 at 17:03
  • 1
    @LokiAstari- Absolutely. However, given that the OP's question was localized to this line, I decided to focus my answer on explaining why the memcpy was a bad idea. Rewriting this entire function to be exception-safe would distract from that particular teaching point. – templatetypedef Jun 08 '12 at 17:11