Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt
  • Unsolved
Collapse
qtws26_logo

The Qt Event of the Year: Qt World Summit 2026

Save Your Spot >
Brand Logo
  1. Home
  2. Qt Development
  3. General and Desktop
  4. Code review
Qt 6.12 LTS is out! See what's new in the release blog

Code review

Scheduled Pinned Locked Moved Solved General and Desktop
24 Posts 6 Posters 5.6k Views 5 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • sierdzioS sierdzio

    @qcoderpro said in Code review:

    • Is it all OK now, please?

    Looks fine to me.

    • When the button "one" is clicked, the string "&One" is shown, including the ampersand!

    Ah, that's bad. Well, then you can probably go back to your original solution :-( Or, there is a chance that sender()->property("text").toString() will return it without ampersand (and no need for a cast!), but I don't know.

    • The destructor is removed because all widgets are children of the top-level widget which is created on the stack, so there's nothing to be deleted manually, hence no memory leak.

    Fine.

    • If we explain what the line below in the "printButton()" slot means in simple language, we would say:
    auto button = qobject_cast<QPushButton*>(sender());
    

    Here "sender" returns a pointer to the object which called the slot (here a QPushButton), then that pointer is cast to <QPushButton*> and finally used to initialize "button". A shade vague!

    It might be unclear to people who do not know Qt, yes. The sender() method is a very special kind of method. But to anybody who has used Qt for a while, this is perfectly understandable.

    Q Offline
    Q Offline
    qcoderpro
    wrote on last edited by
    #12

    @sierdzio

    there is a chance that sender()->property("text").toString() will return it without ampersand (and no need for a cast!), but I don't know.

    With or without a cast, the error below turns up:
    error: calling 'property' with incomplete return type 'QVariant'
    Optimistically, there's a way to solve it and not ruin what we've done from the first place where I used three implementations! :|

    It might be unclear to people who do not know Qt, yes. The sender() method is a very special kind of method. But to anybody who has used Qt for a while, this is perfectly understandable.

    Do you mean that is the exact "sender" as the first parameter in the Q_OBJECT::connection!? Yes, I think!

    sierdzioS 1 Reply Last reply
    0
    • J.HilkJ Offline
      J.HilkJ Offline
      J.Hilk
      Moderators
      wrote on last edited by
      #13

      personally I would prefer multiple lambdas over the use of sender()

      e5abd79e-e6aa-4e2b-9941-864d545d9865-image.png


      Be aware of the Qt Code of Conduct, when posting : https://forum.qt.io/topic/113070/qt-code-of-conduct


      Q: What's that?
      A: It's blue light.
      Q: What does it do?
      A: It turns blue.

      1 Reply Last reply
      1
      • Q qcoderpro

        @sierdzio

        there is a chance that sender()->property("text").toString() will return it without ampersand (and no need for a cast!), but I don't know.

        With or without a cast, the error below turns up:
        error: calling 'property' with incomplete return type 'QVariant'
        Optimistically, there's a way to solve it and not ruin what we've done from the first place where I used three implementations! :|

        It might be unclear to people who do not know Qt, yes. The sender() method is a very special kind of method. But to anybody who has used Qt for a while, this is perfectly understandable.

        Do you mean that is the exact "sender" as the first parameter in the Q_OBJECT::connection!? Yes, I think!

        sierdzioS Offline
        sierdzioS Offline
        sierdzio
        Moderators
        wrote on last edited by
        #14

        @qcoderpro said in Code review:

        error: calling 'property' with incomplete return type 'QVariant'

        #include <QVariant>

        (Z(:^

        Q 1 Reply Last reply
        1
        • sierdzioS sierdzio

          @qcoderpro said in Code review:

          error: calling 'property' with incomplete return type 'QVariant'

          #include <QVariant>

          Q Offline
          Q Offline
          qcoderpro
          wrote on last edited by qcoderpro
          #15

          @sierdzio

          Probably you mean this:

          void dialogTest::printButton()
          {
               btnLabel->setText(sender()->property("text").toString());
          }
          

          Still with the ampersands!

          @J-Hilk

          personally I would prefer multiple lambdas over the use of sender()

          I used both versions below:

          connect(one, &QPushButton::clicked, this, [this]() { btnLabel->setText("&One"); });
           connect(two, &QPushButton::clicked, this,  [this]() { btnLabel->setText(two->text()); });
             
          

          Still with the ampersands!

          1 Reply Last reply
          0
          • Q Offline
            Q Offline
            qcoderpro
            wrote on last edited by qcoderpro
            #16

            Not a bug?
            If it is, how to report that?

            mrjjM 1 Reply Last reply
            0
            • Q qcoderpro

              Not a bug?
              If it is, how to report that?

              mrjjM Offline
              mrjjM Offline
              mrjj
              Lifetime Qt Champion
              wrote on last edited by
              #17

              @qcoderpro
              Hi
              Its not a bug. When the text contains a &, a shortcut is made for it but its not removed and not
              shown. So when you ask for its text() , its still included.

              Q 1 Reply Last reply
              1
              • mrjjM mrjj

                @qcoderpro
                Hi
                Its not a bug. When the text contains a &, a shortcut is made for it but its not removed and not
                shown. So when you ask for its text() , its still included.

                Q Offline
                Q Offline
                qcoderpro
                wrote on last edited by
                #18

                @mrjj
                Hi,

                and not shown. So when you ask for its text() , its still included.

                It's what the programmer needs, to be included but "not shown". In the example above, it's shown, while it mustn't! It can clearly be a bug I think.

                Pl45m4P 1 Reply Last reply
                0
                • Q qcoderpro

                  @mrjj
                  Hi,

                  and not shown. So when you ask for its text() , its still included.

                  It's what the programmer needs, to be included but "not shown". In the example above, it's shown, while it mustn't! It can clearly be a bug I think.

                  Pl45m4P Offline
                  Pl45m4P Offline
                  Pl45m4
                  wrote on last edited by Pl45m4
                  #19

                  @qcoderpro said in Code review:

                  It can clearly be a bug I think

                  The button text contains the & to create the shortcut. So why should btn->text() not return the full text with the ampersand?
                  btn->text() just returns the QString which is the displayed button text (without any pre-processing). Only QPushButton (i.e. its base class QAbtractButton) converts & into the shortcut functionality and for this you need the ampersand in your button text. The text-getter does not know about this, so the actual text with ampersand is returned.

                  Another example:
                  If you inspect HTML with a texteditor, you will see <br> and no line break, because the text editor doesn't interpret this as line break, it just shows the raw UTF8 (or whatever) encoded text.


                  If debugging is the process of removing software bugs, then programming must be the process of putting them in.

                  ~E. W. Dijkstra

                  Q 1 Reply Last reply
                  2
                  • Pl45m4P Pl45m4

                    @qcoderpro said in Code review:

                    It can clearly be a bug I think

                    The button text contains the & to create the shortcut. So why should btn->text() not return the full text with the ampersand?
                    btn->text() just returns the QString which is the displayed button text (without any pre-processing). Only QPushButton (i.e. its base class QAbtractButton) converts & into the shortcut functionality and for this you need the ampersand in your button text. The text-getter does not know about this, so the actual text with ampersand is returned.

                    Another example:
                    If you inspect HTML with a texteditor, you will see <br> and no line break, because the text editor doesn't interpret this as line break, it just shows the raw UTF8 (or whatever) encoded text.

                    Q Offline
                    Q Offline
                    qcoderpro
                    wrote on last edited by
                    #20

                    @Pl45m4

                    With this we have two options: either "not to use shortcuts" or "have the text including the ampersand"! Neither is wanted to me.
                    To me, the code behind "setText" should've been designed so that it could process the text and omit the ampersands when showing.

                    Do you have a better way?

                    1 Reply Last reply
                    0
                    • SGaistS Offline
                      SGaistS Offline
                      SGaist
                      Lifetime Qt Champion
                      wrote on last edited by
                      #21

                      Hi,

                      You have here the text property. You set a text, you retrieve the same text. If you want something else, then it's up to you to a step that does transform the output the way you want it.

                      When setting a property, I am expecting the getter to return the exact same value. The special ampersand handling falls in that case. Since I want to use a shortcut then I will handle that fact in my code. If you want to not have to do that then use a normal text and create a QShortCut by hand.

                      Interested in AI ? www.idiap.ch
                      Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

                      Q 1 Reply Last reply
                      1
                      • SGaistS SGaist

                        Hi,

                        You have here the text property. You set a text, you retrieve the same text. If you want something else, then it's up to you to a step that does transform the output the way you want it.

                        When setting a property, I am expecting the getter to return the exact same value. The special ampersand handling falls in that case. Since I want to use a shortcut then I will handle that fact in my code. If you want to not have to do that then use a normal text and create a QShortCut by hand.

                        Q Offline
                        Q Offline
                        qcoderpro
                        wrote on last edited by
                        #22

                        @SGaist

                        create a QShortCut by hand

                        I found it hard for my example.
                        Instead, used a slot:

                        .h :

                        ...
                        private slots:
                            QString removeAmpersand(QString);
                        ...
                        

                        .cpp:

                        ...
                        connect(one, &QPushButton::clicked, this, [this]() {
                               btnLabel->setText(removeAmpersand(one->text()));
                           });
                        
                           connect(two, &QPushButton::clicked, this, [this]() {
                               btnLabel->setText(removeAmpersand(two->text()));
                           });
                           connect(three, &QPushButton::clicked, this, [this]() {
                               btnLabel->setText(removeAmpersand(three->text())); });
                        }
                        
                        QString DialogTest::removeAmpersand(QString text)
                        {
                            QString res = "";
                            for(auto t : text)
                             if(t != '&')
                                 res += t;
                            return res;
                        }
                        ...
                        

                        Fine?

                        1 Reply Last reply
                        0
                        • SGaistS Offline
                          SGaistS Offline
                          SGaist
                          Lifetime Qt Champion
                          wrote on last edited by
                          #23

                          That's not a slot just a normal method. QString::remove will make your code shorter.

                          Interested in AI ? www.idiap.ch
                          Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

                          Q 1 Reply Last reply
                          1
                          • SGaistS SGaist

                            That's not a slot just a normal method. QString::remove will make your code shorter.

                            Q Offline
                            Q Offline
                            qcoderpro
                            wrote on last edited by qcoderpro
                            #24

                            @SGaist

                            Yes, since it's not used in a connection, it's not a slot. Thanks. Solved.

                            1 Reply Last reply
                            0

                            • Login

                            • Login or register to search.
                            • First post
                              Last post
                            0
                            • Categories
                            • Recent
                            • Tags
                            • Popular
                            • Users
                            • Groups
                            • Search
                            • Get Qt
                            • Unsolved