skr
skr

Reputation: 944

C++ - Proper method to use delete while creating a vector of class objects

I saw a few examples of creating a vector of class objects and many of them uses a pointer and new keyword. However, in many cases the delete is not used to free up memory allocated by new. I would like to know if the following piece of code uses delete properly.

I have a class Marker:

class Marker{

    public:
        Marker(int, float, float);
        int marker_id();

    private:
        int id;
        float mx;
        float my;
};

It's constructor is:

Marker::Marker(int idno, float x, float y){

    //ctor
    id = idno;
    mx = x;
    my = y;
}

I need a vector marker_vec with objects or instances of Marker class. Hence, I wrote the following piece of code:

vector <Marker> marker_vec;

Marker *m = new Marker(last_id, m_x, m_y);
marker_vec.push_back(*m);
delete m;

If I use the above code in a loop to create marker_vec[0] and marker_vec[1], I believe that the delete wouldn't delete them and will only free up the pointer m. Is there any disadvantages for the above method?

Upvotes: 1

Views: 71

Answers (1)

Santiago Varela
Santiago Varela

Reputation: 2237

This piece of code is alright, since when you push_back, the contents referenced by the m pointer will be copied and added as the last element of the vector. You're doing good by deallocating the memory you set properly (for every new there is a corresponding delete).

vector <Marker> marker_vec;

Marker *m = new Marker(last_id, m_x, m_y);
marker_vec.push_back(*m);
delete m;

I just think it's unnecessary for you to use pointers in this case having one type of Marker class and your std::vector of type <Marker>.

I would personally improve the implementation of this code to being statically instantiated. It's simple and cleaner in this case:

vector <Marker> marker_vec;

Marker m(last_id, m_x, m_y);
marker_vec.push_back*m);

However, if you maybe had inheritance like different type of markers:

class HighlighterMarker : public Marker { }; 

and

class PenMarker: public Marker { };

Only then, it'd make sense for you to use dynamic memory and your vector to be declared as:

std::vector <Marker*> marker_vec. This one can store all your references to any type of derived class Marker,

Upvotes: 1

Related Questions