Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. Qt Development
  3. General and Desktop
  4. Code review

Code review

Scheduled Pinned Locked Moved Solved General and Desktop
24 Posts 6 Posters 3.4k 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.
  • 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 Extensions
                        • Unsolved