Recursion on object creation

Nick Shapper

In this screenshot imagine that local MainWindow object is called from another class, i just called it in the same class for simplicity
The problem is when i declare a window object, it simply being created over and over again, thus causing stack overflow
If i declare window as a pointer, i'm getting SIGSEGV.
How do i call a volume method properly? And why is this happening?

JonB

@Nick-Shapper
Hi. Please copy & paste code, not screenshots, it helps people to answer.

Why are you creating a brand new MainWindow window object inside a MainWindow method? If you want to access the current instance use this, e.g. volumeValue = this->volume(); or just volumeValue = volume();.

Nick Shapper

@JonB I said that i'm creating it for a simplicity purposes, just imagine that it's not in a MainWindow class, but a Player class.
Yeah, sorry about screenshot, i was in hurry.
Yes, the method that you described is working, but it's not working from a different class, or it's just me being dumb and not knowing how to properly write the code

Here:


#include "mainwindow.h"
#include "player.h"

Player::Player(QWidget *parent) : QObject(parent),
    player()
{




    mediaPlayer = new QMediaPlayer();

    mediaPlayer->setVolume(this->volume()); // SIGSEGV when this method is invoked
}

Player::~Player()
{
    //delete player;
    //delete mediaPlayer;
}

int Player::volume()
{
    MainWindow window;
    volumeValue = window.getVolume();
    return volumeValue;
}




void Player::setVolume(int volume)
{
    mediaPlayer->setVolume(volume);
}

Christian Ehrlicher

@Nick-Shapper said in Recursion on object creation:

int Player::volume()
{
MainWindow window;
volumeValue = window.getVolume();
return volumeValue;
}

What should this code do? Why do you create a local variable of type MainWindow here?

JonB

@Nick-Shapper said in Recursion on object creation:

@JonB I said that i'm creating it for a simplicity purposes, just imagine that it's not in a MainWindow class, but a Player class.

You will not be want to create a new MainWindow object from the Player constructor, or for that matter from anywhere else.

It's not much use us trying to "imagine" what you really have. Purely at a guess: if you need to access something in an existing MainWindow (or whatever) instance you need the pointer to that instance. Try to understand the difference between accessing an existing object versus creating a new instance, my suspicion is that is where your issue lies.

You should never be including mainwindow.h in any .cpp file other than mainwindow.cpp and you should never be accessing the main window object from another class. Pass a parameter to other classes if required, though this should never be done for a main window. Redesign your code if you think it is necessary for other classes to see anything about the main window, because they should not.

If i declare window as a pointer, i'm getting SIGSEGV.

Declaring something never causes a SIGSEGV. If you are getting one show us the stack trace pane to understand exactly what is happening and where it crashes. In this case, my guess is during the call to MainWindow::volume() from the new instance you incorrectly create, that accesses ui->volumeSlider->.... But that new instance has not had called setupUi(), so ui->volumeSlider == nullptr, hence a SIGSEGV on attempting to dereference.

Nick Shapper

@Christian-Ehrlicher It should get a value of volume from a slider in UI. Look at a volume() in my first post. Basically it's a simple getter function

JonB

@Nick-Shapper said in Recursion on object creation:

It should get a value of volume from a slider in UI. Look at a volume() in my first post. Basically it's a simple getter function

Which you should never do by having a class try to access the main window object. It's just bad design.

But if you are not going to change it to make it good: if you want to access the main window in another class, like you show, pass it as an extra explicit parameter to the other class's constructor, or provide a setter method from the other class so that the main window can set it. But in the latter case you will not be able to access it during the other class's constructor, which you are currently trying to do. If you are only going to access the main window's volume() method, pass that instead of attempting to pass the main window object.

JonB

@Nick-Shapper
Presumably here you want the player to take on the same volume as you have set somewhere in the main window. Do not approach that by trying to read the actual widget in the main window's ui. Instead suggest one of:

If the main window creates the Player, have that export a "volume setter" (or an extra parameter to the constructor) and have the main window call that on the created Player instance passing it the desired volume from its ui.
Create some kind of "settings" object of your own. Have the main window set the volume in that as its UI slider changes. Have the Player object pick up the desired volume from that "settings" object.

Nick Shapper

@JonB Here how it looks in a debugger and a line where it thinks a crash occurred
Also, by " If you are only going to access the main window's volume() method, pass that instead of attempting to pass the main window object." you mean that i should pass a function as a parameter?

And here's also how i create a Player object in MainWindow class

JonB

@Nick-Shapper
The screenshot shows exactly what I expected:

In this case, my guess is during the call to MainWindow::volume() from the new instance you incorrectly create, that accesses ui->volumeSlider->.... But that new instance has not had called setupUi(), so ui->volumeSlider == nullptr, hence a SIGSEGV on attempting to dereference.

This will go away when you correctly no longer try to create any new MainWindow instance.

Also, by " If you are only going to access the main window's volume() method, pass that instead of attempting to pass the main window object." you mean that i should pass a function as a parameter?

No, just pass the value: volume() (not volume). So if you create the Player from MainWindow somewhere you might have:

this->player = new Player(this);
player->setVolume(this->volume());
// or
this->player = new Player(this->volume(), this);

Nick Shapper

@JonB Ok, now i see a problem. I'll try to deduce what you said and somehow redesign my code.
Here's one more question. Is there a page in Qt docs that describes how to properly work with that *ui and how to properly design communication between ui and other objects? Or just a good example from docs will do. I searched but didn't find anything on that matter. It's pretty complicated stuff and should be documented

JonB

@Nick-Shapper
No, there is not (that I am aware of). This is not specifically a Qt issue, it is general design principles that I and others have learned over a lifetime of programming! :) You might look up general "OOP" (object-oriented programming) principles.

Your principle of encapsulating access between classes via getter & setter methods was already good. It's just that it's not something to use to gain access to the main window! That may be a "parent" to other QObjects (e.g. passed as parent to Player's constructor), but the general rule is that parents may access child methods but children should not, and should not need to, access their parent. Try to design Player so that it might be used from anywhere, e.g. it should still function if, say, there were no main window to call.

In any case, the most important lesson to learn here is that passing a pointer to an existing object/instance is quite different from creating a brand new object/instance. Even if your code had not crashed, the result returned by window::volume() on your freshly created MainWindow instance would have borne no relationship to the value of the slider in the actual MainWindow instance already in existence.

Nick Shapper

@JonB said in Recursion on object creation:

@Nick-Shapper
No, there is not (that I am aware of). This is not specifically a Qt issue, it is general design principles that I and others have learned over a lifetime of programming! :) You might look up general "OOP" (object-oriented programming) principles.

Your principle of encapsulating access between classes via getter & setter methods was already good. It's just that it's not something to use to gain access to the main window! That may be a "parent" to other QObjects (e.g. passed as parent to Player's constructor), but the general rule is that parents may access child methods but children should not, and should not need to, access their parent. Try to design Player so that it might be used from anywhere, e.g. it should still function if, say, there were no main window to call.

In any case, the most important lesson to learn here is that passing a pointer to an existing object/instance is quite different from creating a brand new object/instance. Even if your code had not crashed, the result returned by window::volume() on your freshly created MainWindow instance would have borne no relationship to the value of the slider in the actual MainWindow instance already in existence.

Ok, now that makes sense. So basically i gotta design Player so that it would be completely independent and also could be used from anywhere, right?