C++ Smart Pointers in a Vector container

Question

I wrote the classic Shape Polymorphism code, but a little bit different because I'm using a Vector container and smart pointers.

I am not a C++ expert, and I would like to know the following:

1. Is there any memory leak?
2. If I don't call shapes.clear(), is there going to be a memory leak?
3. Is there a better way to use smart pointers and containers?

Code:

#include <iostream>
#include <memory>
#include <vector>
using namespace std;

class Shape {
  public:
    virtual float getArea() = 0;
    virtual ~Shape() {}
};

class Rectangle : public Shape {
  public:
    Rectangle(float w, float h) : width(w), height(h) {}

    float getArea() {
      return width * height;
    }
  private:
    float width;
    float height;
};

class Circle : public Shape {
  public:
    Circle(float r) : radius(r) {}

    float getArea() {
      return 3.141592653589793238462643383279502884 * radius * radius;
    }
  private:
    float radius;
};

void printShapeAreas(vector<shared_ptr<Shape>> &shapes) {
  for(int i = 0; i < shapes.size(); i++) {
    cout << shapes[i]->getArea() << endl;
  }
}

int main(int argc, char** argv) {
  vector<shared_ptr<Shape>> shapes;
  //this works, but you told me that is better to use make_shared
  //=============================================================
  //shapes.push_back(shared_ptr<Shape>(new Rectangle(10, 2)));
  //shapes.push_back(shared_ptr<Shape>(new Rectangle(10, 3)));
  //shapes.push_back(shared_ptr<Shape>(new Circle(2)));
  //shapes.push_back(shared_ptr<Shape>(new Circle(3)));
  //better
  //======
  shapes.push_back(std::make_shared<Rectangle>(10, 2));
  shapes.push_back(std::make_shared<Rectangle>(10, 3));
  shapes.push_back(std::make_shared<Circle>(2));
  shapes.push_back(std::make_shared<Circle>(3));
  printShapeAreas(shapes);
  shapes.clear();//If I don't call shapes.clear(), is there going to be a memory leak?
  return 0;
}

Thank you :)

Answer 1

Your code doesn't contain a memory leak. So your first point is fine.

No, if you don't call shapes.clear(), there will not be a memory leak since std::vector's destructor cleans up the container.

I don't know a specific rule about using shared pointers in a container so I'll skip on your third question.

However, I can offer an improvement for creating std::shared_ptrs:

When you create a shared pointer using its constructor, ie shared_ptr<T>(new T());, the control block or the structure that holds bookkeeping information about that resource, is created separately from the object it points to. This might cause cache misses a lot. However, if you create a shared_ptr by using std::make_shared, the control block is allocated with the object you want, therefore, by keeping them together, you can at least mitigate the cost of cache misses: std::make_shared<T>();. For example: std::make_shared<Circle>(3) is equivalent to writing std::shared_ptr<Shape>(new Circle(3)), but usually better.

Answer 2

1) I don't see a memory leak here. However there is a potential to getting one sooner or later: your shape destructor is not virtual. This means that shapes are always destroyed using the base destructor, instead of the right destructor. I.e. if one of your derived shapes would one day allocate memory, it's destructor that would have to release it wouldn't be called.

2) Nothing will happen: if you don't do a clear(), shapes will get destroyed anyway when main() is left, causing its contend to get destroyed.

3) Maybe, but this one is already very good.