Attributes pointing to a value-CodePudding

I am new in stackoverflow and c . Please forgive me if I am not asking the right question, or the right code format. I am facing this problem where I should create a vector in which the element should be stored on the heap, where there is a pointer pointing to the values.

I would like to make sure to create a vector in which its elements on the heap and the destructor who make sure there is no memory leak.

So far I made the followings:

class Vector {
public:
    Vector(){
        mVe.resize(0);
        Size = 0;
        v =  nullptr;
    }
    Vector(size_t size){
        mVe.resize(0);
        Size = size ;
        v =  nullptr;
    }
    Vector(size_t size, double v){
        mVe.resize(0);
        Size = size;
        v  = &v;
        mVe.push_back(v);
    }
     ~Vector()
     {
         delete v;
     }
private:
    std::vector<double> mVe ;
    size_t Size;
    double* v;
};

CodePudding user response：

This code has several issues. mVe.resize(0); inside the constructors doesn't make sense. Why would you do that when the size is already 0?

Also this v = &v; in the 3rd constructor, you are storing the address of a local variable in v and then deleting it in the destructor which will cause undefined behavior. The std::vector<double> mVe owns the underlying buffer so there is no need to delete anything.

Your class is a wrapper for the STL vector. I guess what you really want to implement is a vector class which does all the allocation and deallocation stuff (using new/delete or possibly even std::unique_ptr<double[]>) and also has a size and a capacity member.

CodePudding user response：

What is the purpose of having a separate Size field? std::vector has its own size() method that you can use when needed. So you can get rid of Size completely.

Also, your constructors taking a size parameter should be calling v.resize(size) instead of v.resize(0). However, std::vector has its own constructors that take size and value parameters, you should be using those constructors instead.

Also, v = &v; is just wrong. In this constructor, v is referring to your input parameter, not your class member. So, you can't assign a double* to a double. You would need this->v = &v; instead, except that will leave the v member as a dangling pointer once the constructor exists. You really need v = mVe.data() instead. But, for that matter, you should just get rid of your v member and use the std::vector::data() method or std::vector::operator[] instead when needed.

And get rid of delete v; completely. You are not allocating the memory with new. The vector owns the memory, it will free the memory for you when itself is destroyed.

Try this instead:

class Vector {
public:
    Vector(){
        v = nullptr;
    }
    Vector(size_t size) : mVe(size) {
        v = mVe.data();
    }
    Vector(size_t size, double value) : mVe(size, value) {
        v  = mVe.data();
    }

private:
    std::vector<double> mVe;
    double* v;
};