CODEWITHSUNDEEP
c++
Karnivaurus
Karnivaurus

Reputation: 24121

Memcpy for converting array to vector

I have a function which I want to use to convert an array of doubles to a vector of doubles:

std::vector<double> ArrayToVector(const double* arr, int length)
{
    std::vector<double> vec(length);
    memcpy(&vec[0], &arr[0], length);
    return vec;
};

But when I run:

int main()
{
    double* x = new double[3];
    x[0] = 2;
    x[1] = 4;
    x[2] = 6;
    std::vector<double> y = ArrayToVector(x, 3);
    for (int i = 0; i < 3; i++)
    {
        std::cout << y[i] << " ";
    }

    return 0;
}

I get the output:

0 0 0

Rather than

2 4 6

Why?

Upvotes: 2

Views: 7250

Answers (4)

Maxim Egorushkin
Maxim Egorushkin

Reputation: 136385

Do not use memcpy to copy into a std::vector, it is less efficient and error prone.

It is less efficient because when you construct or resize a vector it fills new elements with a value-initialized element (unless you provide one), which is 0 for arithmetic types. But that initialization is unnecessary because you then overwrite the values. That initialization may be cheap, but it is not free.

std::vector has a constructor that takes two iterators, as others already mentioned, that copies the input range. This constructor avoids the unnecessary default-initialization before copying.

std::vector also has assign and insert member functions that take two iterators and copy the input range efficiently. v.append(beg, end) is v.insert(v.end(), beg, end).

In my opinion, using memset, memcpy and memmov in C++ code is always a mistake.

These functions are implemented by the standard C library and hence lose input argument type/alignment information (because they take void*). Before switching to the most appropriate SIMD version they need to check the alignment and size of the arguments at run-time. The unaligned beginnings and ends are handled by non-SIMD instructions.

Whereas a C++ compiler knows the alignments and sizes from the type and generates appropriate SIMD instructions inline without those alignment and size checks that C library functions do. Again, those checks may be cheap, but they are not free.

C++ algorithms like std::copy, std::copy_backward, std::fill and container copy functions which take two iterators use these C primitive functions for POD types automatically.

C++ initializer expressions like {} in double buf[N] = {}; do memset for you, but again, in a more efficient and a less error prone manner.

Upvotes: 3

R Sahu
R Sahu

Reputation: 206667

You need to use:

memcpy(&vec[0], &arr[0], length*sizeof(double));

or better yet, use:

int main()
{
    double* x = new double[3];
    x[0] = 2;
    x[1] = 4;
    x[2] = 6;

    std::vector<double> y(x, x+3);    
    for (int i = 0; i < 3; i++)
    {
        std::cout << y[i] << " ";
    }

    return 0;
}

Upvotes: 2

Vlad from Moscow
Vlad from Moscow

Reputation: 311048

memcpy copies bytes. So you have to specify the number of bytes (not the number of doubles) you are going to copy.

memcpy(&vec[0], &arr[0], length * sizeof( double ) );

Nevertheless this approach is bad. It is better to define the vector the following way

std::vector<double> ArrayToVector(const double* arr, int length)
{
    return { arr, arr + length };
}

Or

std::vector<double> ArrayToVector(const double* arr, int length)
{
    std::vector<double> vec( arr, arr + length );
    return vec;
}

Take into account that you need to free allocated memory for the array. You could use for example smart pointer std::unique_ptr for the allocated array.

Upvotes: 1

Borgleader
Borgleader

Reputation: 15916

Your problem is memcpy expect a size in bytes, not number of elements so you need to multiply that third argument by sizeof(double) but really what you should be doing is use the constructor for vector which expects two iterators like so :

std::vector<double> y(x, x + 3);

this way you don't even need to worry about sizeof and it's shorter!

Alternatively you could use std::copy (as mentioned in my comment/the other answer but that's longer for no reason)

Upvotes: 6

Related Questions