Qt Forum

    • Login
    • Search
    • Categories
    • Recent
    • Tags
    • Popular
    • Users
    • Groups
    • Search
    • Unsolved

    Solved Using range-based loop on QVector

    General and Desktop
    3
    6
    3032
    Loading More Posts
    • 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.
    • SeDi
      SeDi last edited by SeDi

      Hi,
      trying to iterate over a QVector with a range-based for like this:

          for (auto p : m_dataSets.at(i)->dataPoints()) {
              drawXShape(painter, zoom*p->x(), p->y(), 5);
          }
      

      gives out a warning:

      c++11 range-loop might detach Qt container (QVector) [clazy-range-loop]
      

      Following this thread I have tried to use qAsConst to solve this:

          for (auto p : qAsConst(m_dataSets.at(i)->dataPoints())) {
              drawXShape(painter, p->x(), p->y(), 5);
          }
      

      Unfortunately this seems impossible:

      error: call to deleted function 'qAsConst'
      candidate function [with T = QVector<DataPoint *>] has been explicitly deleted
      candidate function [with T = QVector<DataPoint *>] not viable: expects an l-value for 1st argument
      error: cannot use type 'void' as a range
      

      So I had to go for the (imho fairly ugly) QVectorIterator:

          QVectorIterator<DataPoint *> iter(dataSets().at(0)->dataPoints());
          while (iter.hasNext()) {
              drawXShape(painter, zoom*iter.peekNext()->x(), iter.peekNext()->y(), 5);
              iter.next();
          }
      

      Am I missing something or is QVector just incompatible with range-based for?

      1 Reply Last reply Reply Quote 0
      • Chris Kawa
        Chris Kawa Moderators last edited by Chris Kawa

        is QVector just incompatible with range-based for?

        It most certainly is compatible. qAsConst is deliberately deleted for r-value references, meaning your dataPoints() function returns temporary. What is its signature? If it returns a copy then you already have an inefficiency and how you construct your loop doesn't matter. It should return either a const reference, in which case you don't need any casting, or a non-const reference, in which case you'd need that qAsConst to avoid detaching.

        JKSH 1 Reply Last reply Reply Quote 5
        • JKSH
          JKSH Moderators @Chris Kawa last edited by JKSH

          @SeDi said in Using range-based loop on QVector:

          Following this thread I have tried to use qAsConst to solve this:

          You are on the right path. You need to use a const QVector with range-for, but as @Chris-Kawa explained qAsConst() can't be used with temporary variables.

          Here's another way to provide a const QVector:

          const auto dataPoints = m_dataSets.at(i)->dataPoints();
          for (auto p : dataPoints) {
              drawXShape(painter, zoom*p->x(), p->y(), 5);
          }
          

          Or, you can make your method return a const QVector (example: const QVector<DataPoint*> DataSet::constDataPoints() const;) and pass that directly into the range-for:

          for (auto p : m_dataSets.at(i)->constDataPoints()) {
              drawXShape(painter, zoom*p->x(), p->y(), 5);
          }
          

          @Chris-Kawa said in Using range-based loop on QVector:

          If it returns a copy then you already have an inefficiency and how you construct your loop doesn't matter. It should return either a const reference...

          Disagree: QVector is copy-on-write so returning a copy only increments an internal refcount which is efficient enough. Returning a copy is always safe; returning a reference can be dangerous in some scenarios.

          Qt Doc Search for browsers: forum.qt.io/topic/35616/web-browser-extension-for-improved-doc-searches

          Chris Kawa 1 Reply Last reply Reply Quote 7
          • Chris Kawa
            Chris Kawa Moderators @JKSH last edited by

            @JKSH said:

            a copy only increments an internal refcount which is efficient enough

            Efficient enough for some cases, not in others. Increasing a refcount atomically is not a lot of work but is still more work than doing nothing, and sometimes that difference matters.

            Returning a copy is always safe. Returning a reference can be dangerous

            I could easily make an argument for the reverse. I've fixed a lot of bugs where someone forgot/didn't think to return a & and scratched their heads why they modify a returned container but the changes don't propagate. It depends. Bugs are possible and common in even the simplest of code.

            JKSH 1 Reply Last reply Reply Quote 3
            • JKSH
              JKSH Moderators @Chris Kawa last edited by

              @Chris-Kawa said in Using range-based loop on QVector:

              @JKSH said:

              a copy only increments an internal refcount which is efficient enough

              Efficient enough for some cases, not in others. Increasing a refcount atomically is not a lot of work but is still more work than doing nothing, and sometimes that difference matters.

              In cases where this matters, then Qt containers should probably be avoided anyway -- and the OP's original question would no longer apply.

              Returning a copy is always safe. Returning a reference can be dangerous

              I could easily make an argument for the reverse. I've fixed a lot of bugs where someone forgot/didn't think to return a & and scratched their heads why they modify a returned container but the changes don't propagate. It depends. Bugs are possible and common in even the simplest of code.

              No arguments about the possibility of bugs, regardless of approach.

              Pros and cons of each, I suppose. I learnt C++ through Qt so I've adopted the Qt API's traditional tendency to prioritize safety and ease-of-use.

              Qt Doc Search for browsers: forum.qt.io/topic/35616/web-browser-extension-for-improved-doc-searches

              1 Reply Last reply Reply Quote 3
              • SeDi
                SeDi last edited by

                Thanks for the insight, guys - absolutely helpful!

                1 Reply Last reply Reply Quote 0
                • First post
                  Last post