→DRY: retrieve caller object

UnitScan

In my interface I have four buttons: each of them is connected to one of these functions which, as you can see, perform the same operations. In order to not write four functions that are essentially the same, I'd like to ask if it is possible write a single function and call the button that called it.
I hope it is clear what my intent is

void MainWindow::onBackgroundColor()
{
    QColor color = QColorDialog::getColor();
        if( color.isValid() )
        {
            settings->setValue("theme/background", color.name());
            performStyles();
            QString bgcolorstylesheet = ("background-color: %1; border: none;");
            bgcolor->setStyleSheet(bgcolorstylesheet.arg(color.name()));
        }
}

void MainWindow::onTextColor()
{
    QColor color = QColorDialog::getColor();
        if( color.isValid() )
        {
            settings->setValue("theme/text", color.name());
            performStyles();
            QString textcolorstylesheet = ("background-color: %1; border: none;");
            textcolor->setStyleSheet(textcolorstylesheet.arg(color.name()));
        }
}

void MainWindow::onBorderColor()
{
    QColor color = QColorDialog::getColor();
        if( color.isValid() )
        {
            settings->setValue("theme/border", color.name());
            performStyles();
            QString bordercolorstylesheet = ("background-color: %1; border: none;");
            bordercolor->setStyleSheet(bordercolorstylesheet.arg(color.name()));
        }
}

void MainWindow::onSliderColor()
{
    QColor color = QColorDialog::getColor();
        if( color.isValid() )
        {
            settings->setValue("theme/slider", color.name());
            performStyles();
            QString slidercolorstylesheet = ("background-color: %1; border: none;");
            slidercolor->setStyleSheet(slidercolorstylesheet.arg(color.name()));
        }
}

Christian Ehrlicher

See QObject::sender()

JonB

@UnitScan
You could use sender() [as I see @Christian-Ehrlicher has just replied], or even QSignalMapper. IMHO at least a lambda with a necessary parameter is nicer --- do you know how to use lambdas, they are common nowadays with Qt signals/slots?

UnitScan

I read that sender() method violates the principles of OO programming. So, the more "correct" alternative would be to use lambda functions?

Christian Ehrlicher

@UnitScan said in →DRY: retrieve caller object:

I read that sender() method violates the principles of OO programming.

Where and why? Using lambdas does more or less exactly the same - it just gives you the pointer in the function callwheras with QObject::sender() you get exact the same within the function.

JonB

@UnitScan
Yes. (Though I'd take admonitions about breaking OO with a pinch of salt :) ) It also doesn't work in all circumstances. IMHO at least, better to connect with a lambda which passes a parameter about the sender. That might be the whole sending object (widget), which is like sender(), or you might just pass what you need (e.g. in your case just a string "background" or "text" for the setValue()), which is less OO-offensive.

@Christian-Ehrlicher

Where and why?

It does say this somewhere in something Qt. Do you really want me to go dig it out...? :)

it just gives you the pointer in the function call

But you do not have to pass the object doing the sending, only what you need from it as a parameter. I have yet to write a lambda connection where I actually pass the sending object.

Note that for all the inbuilt Qt signals/slots, none of them pass the sending object, only a parameter about the relevant state if required.

Matters of opinion of course... :)

UnitScan

Solved:

bgcolor = new QPushButton(editor);
bgcolor->setProperty("value", "theme/background");
    connect(
        bgcolor, &QPushButton::released,
        [this]() { this->onBackgroundColor(bgcolor); }
);

void MainWindow::onColorChanged(QPushButton* caller)
{
    QColor color = QColorDialog::getColor();
        if( color.isValid() )
        {
            settings->setValue(caller->property("value").toString(), color.name());
            performStyles();
            QString bgcolorstylesheet = ("background-color: %1; border: none;");
            caller->setStyleSheet(bgcolorstylesheet.arg(color.name()));
        }
}

If it is possible to improve it further, I gladly accept advice :)

Christian Ehrlicher

@JonB I rather try to avoid a lambda in such a case since it makes the code more unreadable on the connect side. And since it's a private slot it doesn't need to be that fool-proof since I'm the only user of this slot. But yes just a matter of taste.

JonB

@UnitScan said in →DRY: retrieve caller object:

If it is possible to improve it further, I gladly accept advice :)

Within the bounds that is a matter of "taste" whether you pass a widget or just a necessary parameter to the slot. Your onColorChanged() is great, provided it is invoked from QPushButton *, and the caller has placed a suitable property on it. But if, say, you just want to use that code to set a background color from a piece of code you will need to refactor it to produce a secondary function which just takes the desired color property string. Or, you'll have to change it if you decide it invoke it from, say, a combobox. Personally I prefer to make my parameters whatever is the "least" required to make method work. But your/@Christian-Ehrlicher's call.

mrjj

Hi
Just a note about docs Warning
https://doc.qt.io/qt-5/qobject.html#sender
"Warning: This function violates the object-oriented principle of modularity. However, getting access to the sender might be useful when many signals are connected to a single slot."

While it does negatively affect the "loosely coupled" quality of the code, it's more along the line do not use this all over the place and cast sender()
to all types of concrete widgets as it will be hard to reuse.