How do I store objects within another object in C++?

Question

I want to make sure that I'm creating/destroying this object properly...

This is the definition of my Camera object which contains references to Vector objects:

#ifndef CAMERA_H
#define CAMERA_H

#include "vector.h"

class Camera {
 private:
  Vector* location;
  Vector* direction;
  float velocity;
 public:
  Camera();
  ~Camera();
};

#endif

which are created in the constructor:

#include "camera.h"

Camera::Camera() {
  location = new Vector(1.0, 1.0, 1.0);
  direction = new Vector(1.0, 1.0, 1.0);
  velocity = 0.0;
}

Camera::~Camera() {
  delete location;
  delete direction;
}

then, whenever I want a camera object I simply call Camera camera.

Am I correct in assuming that when the variable camera goes out of scope, the destructor will be called and I won't suffer any memory leak?
If I want to remove the variable camera before the scope closes, is it correct to perform delete camera?

There is no reason to use `new` here. Make them automatic storage. — Pubby, Jan 20 '13 at 07:10
@Pubby They won't delete when the constructor is finished? EDIT: That is to say, they won't go out of scope when the constructor is completed? — sdasdadas, Jan 20 '13 at 07:10
Well, the problem right now is that you have a leak if the second `new` throws. Automatic ties their lifetime to the lifetime of `Camera`, meaning they'll be destroyed when `Camera` is destroyed. — Pubby, Jan 20 '13 at 07:12

score 2 · Answer 1 · edited May 23 '17 at 10:24

Yes and Yes(provided Camera was created by calling new).
But you also need to follow the Rule of Three.

Also, it is a good idea to rethink if you really need dynamically allocated members?
remember that by using raw pointers with dynamic memory you are taking responsibility of explicitly doing the memory management for your class which is easy to go wrong with.You are much better off simply using instances rather than pointers:

  Vector location;
  Vector direction;

If you must use pointers atleast use smart pointers instead of raw pointer members.

billz · Accepted Answer · 2013-01-20T07:24:33.537

Am I correct in assuming that when the variable camera goes out of scope, the destructor will be called and I won't suffer any memory leak?

yes

If I want to remove the variable camera before the scope closes, is it correct to perform delete camera?

No, camera is not allocated by new operator, you can not delete it, just leave it until it goes out of scope. Unless call new/delete to force object duration.

Potential memory leak:

In below code, there is a chance to leak memory. if constructs location finishes but direction = new Vector(1.0, 1.0, 1.0); fails and exception is thrown, Camera destructor won't be called thus location memory is leaked.

Camera::Camera() {
  location = new Vector(1.0, 1.0, 1.0);
  direction = new Vector(1.0, 1.0, 1.0);
  velocity = 0.0;
}

A better solution: There is no need to introduce pointer for Vector members. Use automatic storage should be preferred.

class Camera {
 private:
  Vector location;
  Vector direction;
  float velocity;

 public:
  Camera() 
  : location(1.0, 1.0, 1.0), 
    direction(1.0, 1.0, 1.0),
    velocity(0.0f)
  {
  }
};

score 1 · Answer 3 · answered Jan 20 '13 at 07:14

To answer your immediate question, Yes, the destructor will be fired, and yes, you're memory will clean up. But you have not provided protection for incorrect copying, assigning of this object type, and virtual destruction, which can lead to significant problems down the road. More details on this can be found by reading up on The Rule of Three and many, many posts on the subject on SO.

Unless you need dynamic allocation (and you very likely do not) do it like this instead:

class Camera 
{
private:
    Vector location;
    Vector direction;
    float velocity;
public:
    Camera();
};

Camera::Camera() 
    : location(1.0, 1.0, 1.0)
    , direction(1.0, 1.0, 1.0)
    , velocity(0.0)
{
}

No destructor required.

How do I store objects within another object in C++?

3 Answers3