2

I have just switched from Java to C++ and everything is going pretty well so far. The language is kind of hard, but I feel as though I am catching on. I have a question about destructors though, and for this I will supply my current code in hopes that someone could provide some clarification on what and how I should proceed.

I am writing a game in OpenGL and have created Three classes: Vector3D, Dimension3D, and Cuboid. My problem starts like this, my cuboid has an instance of both Vector3D and Dimension3D, which you will soon see. I need to know how (the exact routine) of what happens to my Cuboid class once it is flagged for deletion. Or more precisely, if i need to explicitly destroy both the Vector3D and Dimension3D instance when such an event occurs. I hope I articulated this question adequately enough.

Here are my classes.

(Vector3D.cpp)

#include "Vector3D.h"

Vector3D::Vector3D(){
}

Vector3D::Vector3D( const float& x , const float& y , const float& z ){
    this->x = x;
    this->y = y;
    this->z = z;
}

Vector3D::~Vector3D(){
}

void Vector3D::setX( const float& x ){
    this->x = x;
}

void Vector3D::setY( const float& y ){
    this->y = y;
}

void Vector3D::setZ( const float& z ){
    this->z = z;
}

float Vector3D::getX(){
    return x;
}

float Vector3D::getY(){
    return y;
}

float Vector3D::getZ(){
    return z;
}

(Vector3D.h)

#ifndef NE3_Vector3D_H_
#define NE3_Vector3D_H_

class Vector3D{
public:
    Vector3D();
    Vector3D( const float& , const float& , const float& );
    ~Vector3D();
    void setX( const float& );
    void setY( const float& );
    void setZ( const float& );
    void setPosition( const float& , const float& , const float& );
    float getX();
    float getY();
    float getZ();
    float x;
    float y;
    float z;
private:
    // Private Members Go Here
};

#endif // NE3_Vector3D_H_

(Dimension3D.cpp)

#include "Dimension3D.h"

Dimension3D::Dimension3D(){
}

Dimension3D::Dimension3D( const float& width , const float& height , const float& depth ){
    this->width = width;
    this->height = height;
    this->depth = depth;
}

Dimension3D::~Dimension3D(){
}

void Dimension3D::setWidth( const float& width ){
    this->width = width;
}

void Dimension3D::setHeight( const float& height ){
    this->height = height;
}

void Dimension3D::setDepth( const float& depth ){
    this->depth = depth;
}

float Dimension3D::getWidth(){
    return width;
}

float Dimension3D::getHeight(){
    return height;
}

float Dimension3D::getDepth(){
    return depth;
}

(Dimension3D.h)

#ifndef NE3_Dimension3D_H_
#define NE3_Dimension3D_H_

class Dimension3D{
public:
    Dimension3D();
    Dimension3D( const float& , const float& , const float& );
    ~Dimension3D();
    void setWidth( const float& );
    void setHeight( const float& );
    void setDepth( const float& );
    void setSize( const float& , const float& , const float& );
    float getWidth();
    float getHeight();
    float getDepth();
    float width;
    float height;
    float depth;
private:
    // Private Members Go Here
};

#endif // NE3_Dimension3D_H_

And lastly, my Work in progress Cuboid.cpp and Cuboid.h

(Cuboid.cpp)

#include "Cuboid.h"

Cuboid::Cuboid(){

}

Cuboid::Cuboid(const Vector3D& location, const Dimension3D& dimension){
    this->location = location;
    this->dimension = dimension;
}

Cuboid::~Cuboid(){
    // Do i do delete both location and dimension here?
}

void Cuboid::drawImmediate(){

}

void Cuboid::drawVBO(){

}

(Cuboid.h)

#ifndef NE3_CUBOID_H_
#define NE3_CUBOID_H_

#include "Vector3D.h"
#include "Dimension3D.h"

class Cuboid{

public:
    Cuboid();
    Cuboid( const Vector3D& , const Dimension3D& );
    ~Cuboid();
    void drawImmediate();
    void drawVBO();

    Vector3D location;
    Dimension3D dimension;
private:

};
#endif // NE3_CUBOID_H_

I left a comment in Cuboid.cpp within the destructor. I want to know if I should be deleting the Vector3D and Dimension3D there, and an example showing the best way to do this. IE: Any common conventions that express this functionality.

If my question is not adequate, I will be more than happy to provide further clarification. Also, I am sure that there are other questions like this, however, I need to see it in my own code to fully grasp it. (weird learning style), so please forgive me if this turns into a duplicate.

Krythic
  • 4,184
  • 5
  • 26
  • 67
  • My apologies, I corrected it. – Krythic Feb 28 '14 at 13:04
  • 4
    If you haven't done so yet, I suggest you grab a [good C++ book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) if you really want to learn the language. – Angew is no longer proud of SO Feb 28 '14 at 13:04
  • from where are you learning c++? Which book doesn't explain such basic stuff? – BЈовић Feb 28 '14 at 13:06
  • I use snippets from the internet (how i learned best in java) and simply trying new things. If I have a question I branch out and find a video on said topic...this was just kind of specific and wanted the question answered using my already created code. – Krythic Feb 28 '14 at 13:14

5 Answers5

6

In this particular case you do not need any explicit destruction code.

The reason for this is that you are using direct members:

class Cuboid {
public:
    Vector3D location;
};

This means that the Vector3D object is embedded into the memory of the Cuboid and automatically allocated and released together with the Cuboid.

It would be a different case if you had a pointer to the object as member (e.g. Vector3D *location) instead of the member itself. In that case, you would also have to explicitly allocate memory for the location, as well as explicitly release it in the destructor.

Jonas Schäfer
  • 20,140
  • 5
  • 55
  • 69
4

Since you are just starting with C++, a beginner rule of thumb that you can use is that you explicitly delete only what you explicitly allocate.

Since you haven't allocated the Vector3D yourself (for example via new), you shouldn't delete it.

The situation would have been different, if your code looked like this,

// Cuboid.cpp
Cuboid::Cuboid(){
    location = new Vector3d;
}

Cuboid::~Cuboid(){
    delete location; // now this is necessary
}

// Cuboid.hpp
class Cuboid {
private:
    Vector3D* location; // using a raw pointer here, this can be different for scoped/RAII pointer, unique_ptr
};

Also, consider making the members private, encapsulation is important in C++ as much as it is in Java.


Then as you progress with C++ you will encounter RAII, a slightly more advanced topic, that invalidates the rule of thumb above in the sense that you will use the language constructs, or your own scoped/RAII classes to handle the deallocation for you in a deterministic way. For example, in the following case,

std::unique_ptr<Vector3D> location(new Vector3D);

you won't need to deallocate location yourself, it will be automatically deallocated by the C++ standard library when the unique_pointer will go out of the current scope block, or when the enclosing object, of which location is a member, will be deallocated.

mockinterface
  • 14,452
  • 5
  • 28
  • 49
  • The "new" keyword allocates it on the heap, correct? So then do I only delete when things are allocated on the heap? or is there a situation where I must also delete things manually from the stack? (sorry for being a noob, I am quite used to being spoon-fed in java) – Krythic Feb 28 '14 at 13:11
  • @Krynn "Stack" variables are always handled by the language itself, you never deallocate (or allocate) them manually. The language doesn't use the terms "stack" and "heap", instead it has "dynamic storage duraction" (~ heap) and "automatic storage duration" (roughly ~ stack). These terms give a much better hint of what you need to do. For **automatic** storage duration, nothing ;-) – Angew is no longer proud of SO Feb 28 '14 at 13:14
2

If you allocated the Vector3D object and the Dimension3D object dynamically in the constructor of class Cuboid, then you would need to delete them in the destructor of class Cuboid:

Cuboid::Cuboid(const Vector3D& location, const Dimension3D& dimension){
    this->location = new Vector3D(location);
    this->dimension = new Dimension3D(dimension);
}

Cuboid::~Cuboid(){
    delete location;
    delete dimension;
}

But since this is not the case in your code, you do not need to delete these objects.

Whether you allocate a Cuboid object dynamically with Cuboid* x = new Cuboid(...), or statically with Cuboid x(...), these two member objects (location and dimension) will be implicitly destroyed as soon as the (empty) destructor of class Cuboid is invoked.

barak manos
  • 29,648
  • 10
  • 62
  • 114
1

Cuboid's location and dimension members do not need to be deleted in its constructor. Their destructors are invoked automatically when a Cuboid instance is destructed since you are not explicitly managing their memory allocation, i.e. you didn't use "new" when creating them.

Further, given the code you've shown, both the Vector3D and Dimension3D classes don't need to do any manual resource management in their destructors either. Basically, if you aren't using new to allocate memory for any objects you typically don't need to worry about releasing them.

1

No you do not need to deallocate the members location and dimension as they are automatic objects whose lifetimes is automatically controlled. They will automatically be destroyed when the class Cuboid is destroyed.

It is in fact undefined behaviour to perform a delete on e.g. a pointer to a local automatic object.

As a best practice you should not use delete in C++ (unless you know what you're doing). If you need to dynamically allocate memory (and the containers in the standard library does not do the job) then you should use a smart pointer, as std::shared_ptr, that can handle the deallocation for you (see RAII).

Community
  • 1
  • 1
Felix Glas
  • 15,065
  • 7
  • 53
  • 82