Private slots

[kshegunov]

Hi,
I have been able to make use of "truly" private slots through the pointer to member syntax with lambdas. However, it starts to clutter unrelated parts of my code, which I would want to avoid. Consider for example this constructor:

AedWoodsSaxonOperator::AedWoodsSaxonOperator(QObject * parent)
    : AedOperator(new AedWoodsSaxonOperatorPrivate(this), parent)
{
    Q_D(AedWoodsSaxonOperator);

    auto onNucleusChange = [this, d] (AedNucleus * nucleus) -> void {
        QObject::disconnect(d->nucleonsChanged);
        d->nucleonsChanged = QObject::connect(nucleus, &AedNucleus::nucleonsChanged, [this, d] (quint16 nucleons) -> void {
            d->radius = 1.25 * std::cbrt(nucleons);
        });
    };

    QObject::connect(this, &AedOperator::modelChanged, this, [this, d, onNucleusChange] (AedModel * model) -> void {
        onNucleusChange(model->nucleus());

        QObject::disconnect(d->nucleusChanged);
        d->nucleusChanged = QObject::connect(model, &AedModel::nucleusChanged, this, onNucleusChange);
    });
}

It contains code that's completely unrelated to initialization and I'd love to move it out of there (ideally I'd place it in the private class). So what are my options? Am I stuck to Q_PRIVATE_SLOT and the old syntax?

Thanks in advance.

[VRonin]

Ok, now I get it (or at least I think I do).

you can use the method of the private class the same way you use a lambda (lambda is just a special case of the more general std::function call):

testprivate.h

#include <QObject>
class TestCallSlotPrivate;
class TestCallSlot : public QObject
{
    Q_OBJECT
    Q_DECLARE_PRIVATE(TestCallSlot)
    Q_DISABLE_COPY(TestCallSlot)
public:
    explicit TestCallSlot(QObject* parent = Q_NULLPTR);
    virtual ~TestCallSlot();
protected:
    TestCallSlotPrivate* d_ptr;
};

testprivate.cpp

#include "testprivate.h"
#include <QTimer>
#include <functional>
class TestCallSlotPrivate{
    Q_DECLARE_PUBLIC(TestCallSlot)
    TestCallSlotPrivate(TestCallSlot* q)
        :q_ptr(q)
    {
        Q_ASSERT(q);
        QObject::connect(&m_privateTimer,&QTimer::timeout,q/*just gives context*/,std::bind(&TestCallSlotPrivate::privateSlot,this));
        m_privateTimer.start(300);
    }

    TestCallSlot* q_ptr;
    QTimer m_privateTimer;
    void privateSlot(){
        qDebug("Slot Called");
    }
};



TestCallSlot::TestCallSlot(QObject *parent)
    : QObject(parent)
    , d_ptr(new TestCallSlotPrivate(this))
{
}

TestCallSlot::~TestCallSlot(){
    delete d_ptr;
}

[VRonin]

Probably I got this question completely wrong but Q_DECLARE_PUBLIC(x) contains friend class x; so from the private part can't you access the private slot directly through the q pointer?

[kshegunov]

I think you misunderstand me. I mean the following:
I'd like to move this code into functions/slots inside the private class, not vice versa (it already is in the public class); the problem isn't accessing the public class from the private one. So currently I can attach lambdas but if I go with Q_PRIVATE_SLOT I'm stuck with the old connect syntax (as the private class isn't a QObject derived). Perhaps I'm missing something, hence the question ...

[VRonin]

Ok, now I get it (or at least I think I do).

you can use the method of the private class the same way you use a lambda (lambda is just a special case of the more general std::function call):

testprivate.h

#include <QObject>
class TestCallSlotPrivate;
class TestCallSlot : public QObject
{
    Q_OBJECT
    Q_DECLARE_PRIVATE(TestCallSlot)
    Q_DISABLE_COPY(TestCallSlot)
public:
    explicit TestCallSlot(QObject* parent = Q_NULLPTR);
    virtual ~TestCallSlot();
protected:
    TestCallSlotPrivate* d_ptr;
};

testprivate.cpp

#include "testprivate.h"
#include <QTimer>
#include <functional>
class TestCallSlotPrivate{
    Q_DECLARE_PUBLIC(TestCallSlot)
    TestCallSlotPrivate(TestCallSlot* q)
        :q_ptr(q)
    {
        Q_ASSERT(q);
        QObject::connect(&m_privateTimer,&QTimer::timeout,q/*just gives context*/,std::bind(&TestCallSlotPrivate::privateSlot,this));
        m_privateTimer.start(300);
    }

    TestCallSlot* q_ptr;
    QTimer m_privateTimer;
    void privateSlot(){
        qDebug("Slot Called");
    }
};



TestCallSlot::TestCallSlot(QObject *parent)
    : QObject(parent)
    , d_ptr(new TestCallSlotPrivate(this))
{
}

TestCallSlot::~TestCallSlot(){
    delete d_ptr;
}

[VRonin]

In your case:

AedWoodsSaxonOperator::AedWoodsSaxonOperator(QObject * parent)
    : AedOperator(new AedWoodsSaxonOperatorPrivate(this), parent)
{
    Q_D(AedWoodsSaxonOperator);


    QObject::connect(this, &AedOperator::modelChanged, this, std::bind(&AedWoodsSaxonOperatorPrivate::onModelChanged,d,std::placeholders::_1));
}

void AedWoodsSaxonOperatorPrivate::onNucleusChange(AedNucleus * nucleus){
QObject::disconnect(nucleonsChanged);
nucleonsChanged = QObject::connect(nucleus, &AedNucleus::nucleonsChanged, q_func(), std::bind(&AedWoodsSaxonOperatorPrivate::onNucleonsChanged,this,std::placeholders::_1));
}

void AedWoodsSaxonOperatorPrivate::onNucleonsChanged(quint16 nucleons){
radius = 1.25 * std::cbrt(nucleons);
}

void AedWoodsSaxonOperatorPrivate::onModelChanged(AedModel * model){
onNucleusChange(model->nucleus());
QObject::disconnect(nucleusChanged);
nucleusChanged = QObject::connect(model, &AedModel::nucleusChanged, q_func(), std::bind(&AedWoodsSaxonOperatorPrivate::onNucleusChange,this,std::placeholders::_1));
}

[kshegunov]

Well I am a dumb ass! This should work fine. Thanks, Luca!

[VRonin]

@kshegunov said in Private slots:

Well I am a dumb ass!

You most definitely are not.

Final question. Did you end up using std::bind or did you manage to find a way to use pre-C++11 function pointers in this case?

[kshegunov]

@VRonin said in Private slots:

Did you end up using std::bind or did you manage to find a way to use pre-C++11 function pointers in this case?

Yes, I used std::bind. If I'm not mistaken it creates a wrapper call around the function pointer, right. So:

std::bind(&AedWoodsSaxonOperatorPrivate::onModelChanged,d,std::placeholders::_1)

Would expand to something like:

void (AedWoodsSaxonOperatorPrivate::*ptm)(AedModel *) = &AedWoodsSaxonOperatorPrivate::onModelChanged;
[d, ptm] (AedModel * model) -> void {
    (d->*ptm)(model);
}

Not that I can't do it manually, but I see no reason to bother with it. I use pre-C++11 pointer to member trickery in another place:

class AedWoodsSaxonOperatorPrivate : public AedOperatorPrivate
{
    Q_DECLARE_PUBLIC(AedWoodsSaxonOperator)

    typedef qreal (AedWoodsSaxonOperatorPrivate::*OneBodyMatrixElement)(qint32, qint32);

public:
    // Definition is not provided and will never be. Just a forward declaration for the specializations.
    template <class Basis>
    qreal matrixElement(qint32, qint32);

protected:
    OneBodyMatrixElement oneBodyMatrixElement;
};

Where that one is set like this:

void AedWoodsSaxonOperatorPrivate::basisChanged(AedBasis * basis)
{
    if (dynamic_cast<AedCylindricalBasis *>(basis))
        oneBodyMatrixElement = &AedWoodsSaxonOperatorPrivate::matrixElement<AedCylindricalBasis>;
    else
        oneBodyMatrixElement = nullptr;
}

This is relevant only for the private API (naturally) and the rationale is to forgo a second level of polymorphism and spare myself a virtual call, hopefully improving branch prediction. This one actually matters as it's called millions of times while for the one from the question above I don't care about any efficiency, it's called a handful of times.

PS.
If you're curious, current code can be found here