QT old style connects

[atsju]

I'm using clazy to to update old-style-connect.
More information here and here.
The new syntax is safer because it's checked at compile time. But also it should be marginally faster (I measured no difference).
The clazy automated fix seem to work well but there are about 32 other old style connect that I will fix manually.

I think I found several that should not be working in current code. However I don't know how to test them:

• DFTFringe/oglview.cpp

  Line 223 in db86845

  connect(savePb, SIGNAL(pressed()), sm, SLOT(saveAllContours()));

  
  saveAllContours is a private slot. Can you let me know how to test this specific line ? I don't know what corresponds to OGLview in the GUI.

• DFTFringe/simulationsview.cpp

  Line 169 in db86845

  myMenu.addAction("Save as image", this, SLOT(saveImage()));

  the signature of saveImage(QString fileName) doesn't match QAction::triggered(bool checked = false)

• DFTFringe/rmsplot.cpp

  Line 13 in db86845

  connect(Picker, SIGNAL(selected(QString)),this,SLOT(selectedwave(QString)));

  probably broken since Some code improvements #211 (me).

• DFTFringe/mainwindow.cpp

  Line 184 in db86845

  connect(m_surfTools, SIGNAL(updateSelected()), m_surfaceManager, SLOT(backGroundUpdate()));

  SurfaceManager::backGroundUpdate is a private slot
• ...
• List will increase when I work on it

[githubdoe]

select a couple of wave fronts. Press show selected wave fronts on contour plot. Press the save button. It works for me.

[githubdoe]

Private slots are ok if it is used by the class itself which is probably the case with these.

[githubdoe]

Simulation view: Display ronchi and Foucault view
Right click on foucault or ronchi image to open the right context menu.

[githubdoe]

RMSPlot

Two ways I think. Batch process some igrams. hover over a point on the RMS plot display in the batch processing dialog. Left click that point.

2nd way: load several wave fronts. Select the "filter" button lower right of the wave front list. Hover over a point on the plot should show info. Left click point should select and show the wave front for that point.

[githubdoe]

Mainwindow:

Select several wave fronts. Press the update selected button below the list.

[githubdoe]

You probably already know that I think doing these things is a waist of time. Yes there are better ways to connect now if you remember the syntax of how to write them. Which I never do and just copy a connect perhaps copying an older method when creating a new connect.

However I try to watch the debug output which will inform me of broken connections. That I then fix. So generally all those that clazy complains about are working and do not need to be addressed other than to stop it from complaining.

It seems to me that the amount of good clazy provides is far outweighed by the amount of noise it creates when reviewing old existing code that works. I probably said that already.

[atsju]

Hi Dale.
Thank you for the insights, I will have a look.
I know you think it's a waste of time and I try to look at things that could eventually be broken without us knowing. I also acknowledged you are very careful and most warnings are not a problem thanks to this. On the other side I have also noticed a few problems in the past.
Anyway, rest assured I like to have you moderate me and if you don't want some change I have no problem you explicitly refuse a pull request.

About changing to new connect specifically, I see it as an investment for the futur. Compile time checks means you no longer need to check the console for missed connects (except for the connect by name on_something). Let's wait and see if I notice an existing problem during the process.
Additional note about the private slots: I confirm those are used by an other class. So if they are working it could be a flaw in Qt or something I haven't understood yet. I will investigate and let you know.

[githubdoe]

Yes the new connect is to be preferred. I just have trouble finding examples of the exact syntax when I write one.

Perhaps if the connect is created by the class owing the private then all is ok. But I would expect the compiler to catch any outside the class. I know it has in the past.

[atsju]

I checked and confirm the old style connect just ignores the private keyword of slots. So those ones work even if they shouldn't.
Why

• Old-style connects are implemented using string-based lookups (e.g. "2triggered(bool)" → "1mySlot()"), not compile-time type checking.
• At runtime, Qt looks up the slot name in the moc-generated meta-object, and if found, it connects.
• Since moc generates meta-data for all slots (public, protected, private), they are all equally accessible to the signal–slot system.

For the syntax, it's actually quite simple. You might want to store this somewhere:

here is an example

QPushButton *savePb = new QPushButton("Save as Image",w);
SurfaceManager *sm = SurfaceManager::get_instance();
connect(savePb, SIGNAL(pressed()), sm, SLOT(saveAllContours()));

becomes

// remove SIGNAL/SLOT, remove parenthesis => add &class::
connect(savePb, &QPushButton::pressed, sm, &SurfaceManager::saveAllContours);

In the case where you have overloaded signals or slots you need to disambiguate

// add QOverload<>::of() and inside the <> you add the signature of the version you want
SIGNAL(selected()) //from QwtPlotPicker
//becomes one of those
QOverload<const QPointF &>::of(&QwtPlotPicker::selected)
QOverload<const QRectF &>::of(&QwtPlotPicker::selected)
QOverload<const QVector< QPointF > &>::of(&QwtPlotPicker::selected)

For simple cases, the new syntax is actually as simple and clearer to read ad you know immediately the class it's referring to.
I agree the QOverload is a bit more tedious but on the other hand you know exactly what is called in the end.

[githubdoe]

Ah, that makes sense about private being ignored. Yes maybe I can remember the new syntax. When put they you did. I have seen the disambiguate error before and did not understand the problem. So just went to the old connect method which worked.