Is this a good programming practice to cleanup memory?

Question

I know that this question is probably opinion oriented, but since I am an amateur programmer I want to know whether this is a good practice to clean up the memory.
To clear my objects I use static QVector in which I store the pointers to the objects created and I made a static function called Perfrom_Cleanup which deletes all the objects from the pointers stored in the vector. Here is my code, to get a clearer idea.

Header File

#ifndef AUTOMATIC_CLEANUP_H
#define AUTOMATIC_CLEANUP_H

#include <QObject>
#include <QVector>
#include <QDebug>

class Automatic_Cleanup : public QObject
{
    Q_OBJECT

    static QVector<Automatic_Cleanup*> m_Objects;
public:
    explicit Automatic_Cleanup(QObject *parent = nullptr);
    ~Automatic_Cleanup();

    static void Perfrom_Cleanup();

signals:

public slots:
};

#endif // AUTOMATIC_CLEANUP_H

CPP File

#include "Automatic_Cleanup.h"

QVector<Automatic_Cleanup*> Automatic_Cleanup::m_Objects; 

Automatic_Cleanup::Automatic_Cleanup(QObject *parent) : QObject(parent)
{

    m_Objects.append(this);
}

Automatic_Cleanup::~Automatic_Cleanup()
{
    qDebug()<<"Deleting object "<<this->objectName();
    int i = m_Objects.indexOf(this, 0);
    if(i<0)
        return;

    m_Objects.remove(i);
}

void Automatic_Cleanup::Perfrom_Cleanup()
{
    // Deleting from last
    for(int i=m_Objects.count()-1; i>=0; i--){
        Automatic_Cleanup *obj = m_Objects.at(i);
        delete obj;
    }
}

From my main cpp file I am doing this, to verify

for(int i=0; i<10; i++){
    Automatic_Cleanup *obj = new Automatic_Cleanup;
    obj->setObjectName(tr("Object %1").arg(i+1));

    if(i==4)
        delete obj;
}

Automatic_Cleanup::Perfrom_Cleanup();

not soo much about opinions actually. You should either use automatic storage or smart pointers. `new` and `delete` have no place in modern C++ (unless you write libraries that need to do memory managment) — 463035818_is_not_an_ai, Sep 26 '18 at 09:50
Why not store the objects in the vector? They would be cleaned up when vector is destroyed. — Yksisarvinen, Sep 26 '18 at 09:51
It is extremely *bad* practice *not* to clean up your memory. — Bathsheba, Sep 26 '18 at 09:52
If you have custom-defined destructors, it's not only about memory cleanup. — mip, Sep 26 '18 at 10:39
@user463035818 Qt uses parent-child relationships to manage memory (this was designed long before smart pointers became a part of standard library), so `new` actually has to be used with Qt. — mip, Sep 26 '18 at 10:46
@Yksisarvinen Unfortunately `QObject` is uncopyable and unmovable. — mip, Sep 26 '18 at 10:53
Check [this answer](https://stackoverflow.com/a/47107116/5366641) out. — scopchanov, Sep 26 '18 at 11:02

score 9 · Accepted Answer · edited Sep 26 '18 at 16:59

9

In Qt memory management is organized by parent-child relation. Basically when you create a new QObject of any kind, and give it a parent argument, parent will be responsible for deleting that object.

So basically, this static vector is obsolete and this is not an opinion. When parent-child relation is used if you delete root element all children will be destroyed.

This static vector is kind of global variable and this should be avoided in any framework or programming language. So this is another reason not to use it.

edited Sep 26 '18 at 16:59

Mike

8,055
1
30
44

answered Sep 26 '18 at 09:56

Marek R

32,568
6
55
140

Plus, if you do it like poster, you might end up with null-reference-errors, since the parent might not know it was deleted – Amfasis Sep 26 '18 at 09:57
2

Nope. When child is deleted, parent is notified and parent will not try delete this again. This is done by Qt design. – Marek R Sep 26 '18 at 09:59
Unless you have a member pointer. I mean, I feel like I would end up in that situation, unless there is some trick I don't know? – Amfasis Sep 26 '18 at 10:02

Mike · Answer 2 · 2018-09-26T18:55:31.897

This approach has too many downsides, many of them have been already mentioned.

I would like to additionally note that you are only cleaning up your memory at the end of your main function (most modern operating systems clean up anyway when your process exits^*).

A more serious need for this clean-up emerges while running your program, for instance, when your program has allocated too many objects and still needs more (e.g. imagine that you are writing a service that should run for weeks or maybe months). Your approach wouldn't be of any help in this case.

You are basically implementing a garbage collector that kicks in only when the program is about to exit, and works for a specific hierarchy of types.

^{* please note that I am not encouraging the delegation of any form of clean-up from the process to the operating system.}

Is this a good programming practice to cleanup memory?

2 Answers2