Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. General talk
  3. The Lounge
  4. Recurring C++ and Qt anti-patterns
Forum Updated to NodeBB v4.3 + New Features

Recurring C++ and Qt anti-patterns

Scheduled Pinned Locked Moved The Lounge
126 Posts 17 Posters 71.1k Views 10 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.
  • Chris KawaC Offline
    Chris KawaC Offline
    Chris Kawa
    Lifetime Qt Champion
    wrote on last edited by Chris Kawa
    #51

    Yeah, it was a bit out of topic. Here's one anti-pattern I encounter something like 9/10 code reviews:

    auto widget = new SomeWidget(some_widget);
    auto layout = new SomeLayout(some_other_widget);
    layout->addWidget(widget);
    

    It's not a correctness bug. It's a subtle performance one. Compare this with:

    auto widget = new SomeWidget();
    auto layout = new SomeLayout();
    layout->addWidget(widget);
    some_other_widget->setLayout(layout);
    

    If you don't see it - count how many times parents need to be changed and imagine there's not one but, say, 50 widgets and layouts.
    For extra sweetness do the same when the parent widget is visible - how many times layouts need to be recalculated?

    jsulmJ 1 Reply Last reply
    6
    • Chris KawaC Chris Kawa

      Yeah, it was a bit out of topic. Here's one anti-pattern I encounter something like 9/10 code reviews:

      auto widget = new SomeWidget(some_widget);
      auto layout = new SomeLayout(some_other_widget);
      layout->addWidget(widget);
      

      It's not a correctness bug. It's a subtle performance one. Compare this with:

      auto widget = new SomeWidget();
      auto layout = new SomeLayout();
      layout->addWidget(widget);
      some_other_widget->setLayout(layout);
      

      If you don't see it - count how many times parents need to be changed and imagine there's not one but, say, 50 widgets and layouts.
      For extra sweetness do the same when the parent widget is visible - how many times layouts need to be recalculated?

      jsulmJ Offline
      jsulmJ Offline
      jsulm
      Lifetime Qt Champion
      wrote on last edited by
      #52

      @chris-kawa said in Recurring C++ and Qt anti-patterns:

      auto widget = new SomeWidget(widget);

      I hope this is a typo :-)

      https://forum.qt.io/topic/113070/qt-code-of-conduct

      Chris KawaC 1 Reply Last reply
      1
      • jsulmJ jsulm

        @chris-kawa said in Recurring C++ and Qt anti-patterns:

        auto widget = new SomeWidget(widget);

        I hope this is a typo :-)

        Chris KawaC Offline
        Chris KawaC Offline
        Chris Kawa
        Lifetime Qt Champion
        wrote on last edited by
        #53

        @jsulm Sure, sorry, fixed :)

        1 Reply Last reply
        0
        • Kent-DorfmanK Offline
          Kent-DorfmanK Offline
          Kent-Dorfman
          wrote on last edited by
          #54

          @chris-kawa said in Recurring C++ and Qt anti-patterns:

          It's not a correctness bug. It's a subtle performance one. Compare this with:

          It took me a few minutes but ok, I'm convinced... LOL

          1 Reply Last reply
          0
          • fcarneyF Offline
            fcarneyF Offline
            fcarney
            wrote on last edited by
            #55
            mtime.toString(tr("M/d/yyyy hh:mm AP"));
            

            C++ is a perfectly valid school of magic.

            aha_1980A 1 Reply Last reply
            2
            • fcarneyF fcarney
              mtime.toString(tr("M/d/yyyy hh:mm AP"));
              
              aha_1980A Offline
              aha_1980A Offline
              aha_1980
              Lifetime Qt Champion
              wrote on last edited by
              #56

              Hi @fcarney,

              mtime.toString(tr("M/d/yyyy hh:mm AP"));

              OMG. Yeah, that's a good (bad) one :)

              Qt has to stay free or it will die.

              1 Reply Last reply
              0
              • fcarneyF Offline
                fcarneyF Offline
                fcarney
                wrote on last edited by
                #57

                Just created this pattern today:

                if(condition == somevalue)
                somestatement.append(whatever);
                

                I forgot the indentation so it didn't look like and if statement.
                I I know this is really simple and not an error/bad practice. It is more a readability issue.

                Going to be more rigorous in the future:

                if(condition == somevalue){
                    somestatement.append(whatever);
                }
                

                C++ is a perfectly valid school of magic.

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

                  Well, that one made a big security hole in Apple's authentication code but it was the other way around, several lines under the if without curly brackets.

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

                  aha_1980A 1 Reply Last reply
                  3
                  • fcarneyF Offline
                    fcarneyF Offline
                    fcarney
                    wrote on last edited by
                    #59

                    Wow, okay, I take it back. Bad pattern!

                    C++ is a perfectly valid school of magic.

                    1 Reply Last reply
                    0
                    • SGaistS SGaist

                      Well, that one made a big security hole in Apple's authentication code but it was the other way around, several lines under the if without curly brackets.

                      aha_1980A Offline
                      aha_1980A Offline
                      aha_1980
                      Lifetime Qt Champion
                      wrote on last edited by
                      #60

                      @SGaist I've seen GCC 7.3 warning about exactly this problem ("thif if clause ... does not guard ...").

                      So hopefully such problems will go away sooner than later.

                      Regards

                      Qt has to stay free or it will die.

                      kshegunovK 1 Reply Last reply
                      0
                      • aha_1980A aha_1980

                        @SGaist I've seen GCC 7.3 warning about exactly this problem ("thif if clause ... does not guard ...").

                        So hopefully such problems will go away sooner than later.

                        Regards

                        kshegunovK Offline
                        kshegunovK Offline
                        kshegunov
                        Moderators
                        wrote on last edited by
                        #61

                        Nope. It warns if it doesn't actually guard (as if you had put incidentally ; at the end of the if):

                        if (something)
                        something else;
                        something else else; //< Can't warn about that
                        

                        Read and abide by the Qt Code of Conduct

                        aha_1980A 1 Reply Last reply
                        0
                        • aha_1980A Offline
                          aha_1980A Offline
                          aha_1980
                          Lifetime Qt Champion
                          wrote on last edited by aha_1980
                          #62

                          OMG

                          QString CharToString(char *str)
                          {
                              QString result = "";
                              int lengthOfString = strlen(str);
                          
                              QString s;
                              for(int i = 0; i < lengthOfString; i++)
                              {
                                  s = QString("%1").arg(str[i], 0, 16);
                          
                                  if(s.length() == 1)
                                      result.append("0");
                          
                                  result.append(s);
                              }
                          
                              return result;
                          }
                          

                          There are multiple anti-patterns in that function (who finds all?!), but basically the solution is to use: QString s = QByteArray::toHex(str);

                          Qt has to stay free or it will die.

                          1 Reply Last reply
                          2
                          • kshegunovK kshegunov

                            Nope. It warns if it doesn't actually guard (as if you had put incidentally ; at the end of the if):

                            if (something)
                            something else;
                            something else else; //< Can't warn about that
                            
                            aha_1980A Offline
                            aha_1980A Offline
                            aha_1980
                            Lifetime Qt Champion
                            wrote on last edited by
                            #63

                            @kshegunov

                            What I meant was the following example:

                            8ac16113-6178-40b1-9ce0-232de08d5cb6-image.png

                            which is already an improvement on poorly formatted code.

                            Regards

                            Qt has to stay free or it will die.

                            1 Reply Last reply
                            2
                            • fcarneyF Offline
                              fcarneyF Offline
                              fcarney
                              wrote on last edited by
                              #64

                              Including the wrong header that doesn't match cpp file definition... This is really confusing.

                              C++ is a perfectly valid school of magic.

                              1 Reply Last reply
                              0
                              • fcarneyF Offline
                                fcarneyF Offline
                                fcarney
                                wrote on last edited by
                                #65

                                Hmmm... I have just found some icky syntax that makes me think it is an anti-pattern just cause its icky:

                                #include <vector>
                                
                                template <class T>
                                class IteratorClass
                                {
                                public:
                                    std::vector<int>::iterator end(); // neat syntax
                                    std::vector<T>::iterator end2(); // error, needs typename
                                    typename std::vector<T>::iterator begin(); // really? this is getting ugly
                                    using retIterator = typename std::vector<T>::iterator; // ugly
                                    typedef typename std::vector<T>::iterator retIterator; // fugly
                                
                                private:
                                    std::vector<T> m_data;
                                };
                                

                                Got stuck on this last night and just couldn't figure out why the simplest syntax would not work with templates. My IDE even told me I needed "typename" and I kept trying "typedef" (💩). Yeah, it was not a good time to be coding, so I went to bed. 😀

                                C++ is a perfectly valid school of magic.

                                1 Reply Last reply
                                0
                                • Kent-DorfmanK Offline
                                  Kent-DorfmanK Offline
                                  Kent-Dorfman
                                  wrote on last edited by
                                  #66

                                  @fcarney said in Recurring C++ and Qt anti-patterns:

                                  std::vector<T>::iterator end2(); // error, needs typename

                                  Yeah, this being an error (inside a template definition) really bugs me as well.

                                  I'm sure there is a perfectly good brainiac reason it barfs, but I could really see myself wanting to do something like this, as "end2" instead of end2()

                                  fcarneyF 1 Reply Last reply
                                  0
                                  • Kent-DorfmanK Kent-Dorfman

                                    @fcarney said in Recurring C++ and Qt anti-patterns:

                                    std::vector<T>::iterator end2(); // error, needs typename

                                    Yeah, this being an error (inside a template definition) really bugs me as well.

                                    I'm sure there is a perfectly good brainiac reason it barfs, but I could really see myself wanting to do something like this, as "end2" instead of end2()

                                    fcarneyF Offline
                                    fcarneyF Offline
                                    fcarney
                                    wrote on last edited by
                                    #67

                                    @Kent-Dorfman
                                    Apparently its a "dependent name":
                                    https://en.cppreference.com/w/cpp/language/dependent_name

                                    I have not taken time to understand it, but there is the "reason".

                                    C++ is a perfectly valid school of magic.

                                    1 Reply Last reply
                                    0
                                    • fcarneyF Offline
                                      fcarneyF Offline
                                      fcarney
                                      wrote on last edited by
                                      #68

                                      I think I may need to stop coding in the evening. I ran into a weird bug that I cannot duplicate today:

                                      #include <vector>
                                      
                                      template<class T>
                                      class SomeObject
                                      {
                                          using Storage = std::vector<T>;
                                      public:
                                          SomeObject(size_t len){
                                              m_data.resize(len);
                                          }
                                      
                                          size_t getSize(){
                                              return m_data.size();
                                          }
                                      
                                      private:
                                          Storage m_data;
                                      };
                                      
                                      class UsesSomeObject
                                      {
                                      public:
                                          UsesSomeObject()
                                              : m_someval(0)
                                              , m_somedata(128) // if not initialized the whole object was spitting out weird data
                                          {
                                      
                                          }
                                      
                                      private:
                                          int m_someval;
                                          SomeObject<int> m_somedata;
                                      };
                                      

                                      I don't know if this had anything to do with templates or not. I was working with one at the time. There is a comment in the above code about not initializing m_somedata. I didn't have a default constructor or maybe it created one for me (not sure). Accessing the vector internal to the class had all sorts of "interesting" behavior. Then when I realized my error everything started working fine. It was just a very sneaky issues. However, on my compiler at work it is not letting me compile this. So I am not sure of the situation where it would let me compile this. Maybe if it creates its own default constructor. The lesson is make sure everything is getting initialized before using them!

                                      I will check tonight to see if I can simplify the actual condition that caused this. It was quite interesting and the errors didn't match the source of the problem.

                                      C++ is a perfectly valid school of magic.

                                      1 Reply Last reply
                                      0
                                      • fcarneyF Offline
                                        fcarneyF Offline
                                        fcarney
                                        wrote on last edited by
                                        #69

                                        Not an antipattern, just disappointing. I cannot do this:

                                        std::vector<float&> frefs; 
                                        

                                        I know why. I know you can use std::reference_wrapper, but it is kinda messy to me.

                                        C++ is a perfectly valid school of magic.

                                        1 Reply Last reply
                                        0
                                        • Kent-DorfmanK Offline
                                          Kent-DorfmanK Offline
                                          Kent-Dorfman
                                          wrote on last edited by
                                          #70

                                          I guess I don't have a problem with it because in the cases where I might ever consider such an abomination there are always pointers...yes, always pointers.

                                          fcarneyF 1 Reply Last reply
                                          2

                                          • Login

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