[Solved] Keeping a menubar for repeated use in QMainWindow::setMenuBar

raven-worx

The menubar itself is just one class instance. Creating the actions requires allocating many more class instances, so just economizing on one allocate/delete, that of one QMenuBar, does not seem significant.

i never said delete the actions. Unlike with the menubar the ownership of QActions isn't transferred to the widget when you call addAction().

So just create every instance once and compose them together as you need.

In any case, I'm not at all sure that clear() will not also crash. It seems to me that calling setMenuBar then clear may cause clear to crash on freeing memory that was already freed. Calling clear then setMenuBar may cause setMenuBar to crash.

what makes you think that?!

Chris Kawa

It would be nice if there was takeMenuBar(), but that's not a big problem. It's 2 lines of code.
Before you set new bar make sure to set the parent of the current one to null i.e.

if(mainWindow->menuBar())
   mainWindow->menuBar()->setParent(nullptr);
mainWindow->setMenuBar(anotherBar);

One thing to note is that with this you need to keep track of the lifetime of the bars because the unused ones are no longer managed by a parent. This is as easy as putting them in some container and calling qDeleteAll(barsContainer) at the end.

raven-worx

@Chris-Kawa
this is not a matter of QObject parent-child relationship.
The mainwindow keeps the menubar as a pointer and deletes it when a new one is set.

Chris Kawa

@raven-worx Setting a null parent "detaches" the bar from main window. It won't be deleted by the consecutive setMenuBar(). I'm not guessing. I've checked ;)

raven-worx

@Chris-Kawa
i see. Because it's contained in a layout.
Sry for that.

Harry123

@Chris-Kawa
That's a very simple solution to the problem.
Thank you, I'll be trying it out very soon.

kshegunov

@raven-worx, @chris-kawa
http://code.qt.io/cgit/qt/qtbase.git/tree/src/widgets/widgets/qmainwindow.cpp?h=5.5#n517
I'm sorry to argue, but look up here. Line 535 explicitly deletes the menu!

Chris Kawa

@kshegunov That's true, but note that deletion happens in if (topLayout->menuBar() .... block.
When you set parent of the bar to nullptr it is taken out of the layout. But don't take my word for it, check:

qDebug() << mainWindow->layout()->menuBar();  //prints out some pointer
mainWindow->menuBar()->setParent(nullptr);
qDebug() << mainWindow->layout()->menuBar();  //prints 0, the bar was taken out

kshegunov

@Chris-Kawa
Ah! Yes, you're correct of course. This was my initial idea (reparenting the menu bar) but I discarded it because I didn't realize that the menu will be taken out of the layout, silly me! :)

Kind regards.

Harry123

Solution verified as simple and perfectly working.

What I did was to override QMainWindow::setMenuBar as follows :

void myMainWindow::setMenuBar(QMenuBar * bar)
{
  if (menuBar())
    menuBar()->setParent(0);      // detach current menubar so it won't get deleted
  QMainWindow::setMenuBar(bar);
}

raven-worx

@Harry123
please be advised that this only works as long as the instance is of the type myMainWindow, since setMenuBar() is not virtual.

Meaning the following wont work, even the object behind the pointer has the correct type:

QMainWindow* mw = new myMainWindow;  // pointer-type is different from instance-type!
mw->setMenuBar(...);

Harry123

@raven-worx

I have "mw = this;" in the constructor for myMainWindow, where mw is a global variable, and I can now successfully do "mw->setMenuBar(..)" from several other classes with no problem.

It currently does work - why shouldn't it ?

Chris Kawa

@Harry123 It might work for your case but is very dangerous in general. As @raven-worx mentioned setMenuBar() is not virtual. This means that if anyone calls it through a pointer to base class the base class version will be called (which deletes the bar).
In general you can't guarantee that your class will only by accessed via pointer to derived class and in fact you should never assume that.
Example:

QMainWindow* gotcha = mw;
mw->setMenuBar(someBar); //ok
gotcha->setMenuBar(someOtherBar); //someBar is deleted!

Long story short - don't override non-virtual methods. Create a new one with descriptive name.

Harry123

@Chris-Kawa
I don't have this gotcha - my variable is of type myMainWindow*.
This can only not work if there is a bug in the C++ compiler.
But perhaps I will rename it, just in case.

Chris Kawa

@Harry123 said:

This can only not work if there is a bug in the C++ compiler.

As I said - it works here and now for you. But who's to say in a month or a year another person will for whatever reason create a local variable of type QMainWindow* and copy your pointer. The point is that because this method is not virtual a different method will be called depending on the type of pointer you access it through. That's a time bomb type of bug. It does not explode here and now for you but it will for someone else or future you.

Harry123

@Chris-Kawa
Totally agree.