-3

I have a portion of code that I tested and works, and now I would like to make it a class in order to have something cleaner. The following code creates a scene containing a rectangle and displays it on the widget 'graphicview'

QGraphicsScene *scene = new QGraphicsScene;
QGraphicsRectItem *rect = new QGraphicsRectItem();
rect->setRect(0,0,100,100);
scene->addItem(rect);
ui->graphicsView->setScene(scene);

What I would like now is to create a class that contains this scene, so that I just have to call :

MyClass *myscene = new MyClass;
ui->graphicsView->setScene(myscene->scene)

The quetion is, in the class MyClass, shall I have a private argument declared as QGraphicsScene *scene = new QGraphicsScene;, or just a private argument QGraphicsScene *scene and then within the constructor *scene=new QGraphicsScene

And the same for where I should put the delete, within the destructor of MyScene?

edit : Based on answer, I tried to rework my code without new:

QGraphicsScene scene;
QGraphicsRectItem rect;
rect.setRect(0,0,100,100);
scene.addItem(&rect);
ui->graphicsView->setScene(&scene);

But this code doesn't work (widget 'graphicView' displays nothing), and all the example I found of Qt use the new operator. What I am missing ?

sayanel
  • 185
  • 1
  • 7
  • You shouldn't put `new`s or `delete`s anywhere. Instead, you should open your C++ book to the chapter that explains how to use smart pointers, and use them instead. – Sam Varshavchik Apr 22 '19 at 12:34
  • 1
    What's wrong with just `MyClass myscene;`? – stark Apr 22 '19 at 12:39
  • Well I don't know if anything is wrong, but [Qt documentation](https://doc.qt.io/qt-5/qtwidgets-graphicsview-diagramscene-example.html) use them and I'm not at the point where I think they are wrong – sayanel Apr 22 '19 at 12:50

2 Answers2

2

Avoid (Raw) pointers as much as your performance and clarity considerations allow. For example, In your code, It's not necessary to use pointers. Instead, define your scene as a member of your class:

class MyCalss
{
...
  QGraphicsScene scene;
};

And then set it for a view:

MyClass myscene;

ui->graphicsView->setScene(&myscene.scene);

So, from now on, before using pointers, ask this question from yourself, Is it necessary to use pointers? If you have to use pointers, put smart pointers in your considerations too.

s4eed
  • 6,033
  • 7
  • 51
  • 91
  • Thanks for the answer. I based my exemple on this [Qt documentation](https://doc.qt.io/qt-5/qtwidgets-graphicsview-diagramscene-example.html) ('MainWindow Class Implementation'), shall I conclude that Qt does not use good programing practices and I shouldn't use their examples? – sayanel Apr 22 '19 at 13:04
  • Qt is para-C++, they need a meta-compiler to exist (MOC). Any object derived from QObject form a tree so that you don't need to `delete`. That was clever and powerfull when it was invented, since old C++ hadn't better options. But now, it sounds archaic compared to Modern C++. But Qt is still quite OK. – Sandburg Apr 22 '19 at 13:19
  • @sayanel Qt's examples are great. But you'd better improve your C++ knowledge. Learn from examples what they want to create, not HOW. Secondly, it's better to accept an answer, if it answered your question. [It's good stackoverflow practice](https://stackoverflow.com/help/someone-answers) :D ;) – s4eed Apr 23 '19 at 01:24
  • @s4eed well I tried to implement your answer, but I didn't manage to (see my edit on the question), so it is not closed for now – sayanel Apr 23 '19 at 15:34
  • @sayanel I guess that you defined your edited code inside your constructor. So after finishing your constructor, your scene will be destroyed. Define your scene a member of the class. The scene has to be alive as long as your view is alive. So you have to `new` and delete it in destructor, or make it a member of the class. – s4eed Apr 24 '19 at 08:33
0

There is no "specific" place to put new and delete. It entirely depends on where you need a dynamic allocation of memory (new) and once the job od dynamically allocated memory(the variable) is over you need to free that (delete) keeping in mind that the variable won't be used anywhere else as it would lead to undesired behavior.

If at every object creation of a class, the class variable needs to be dynamically allocated right from starting, do it in constructor. Similarily if the variable is used throughout object lifetime delete it in destructor. This is just a very high level idea. I would suggest reading this and also reading more about pointers (smart pointers) and dynamic memory allocation

Valgrind1691
  • 144
  • 1
  • 10