Problem when returning a QVector from a function

[wysota]

Look at line 3 of your code, you are making a copy of the vector there. You should be getting a warning from the compiler about returning a reference to a local object as well. By the way, your find method is not very optimal.

[meazza]

Not getting any warnings regarding this. I tried to change the function without the the code on line 3.

void QDSMainWindow::deleteWire(QDSWireObject *wire)
{
     if(!findObjectVector(wire).isEmpty())
     {
         Q_FOREACH(QDSWireObject* item, findObjectVector(wire))
         {
             if(item == wire)
             {
                 int index = findObjectVector(wire).indexOf(wire);
 
                 if(item->getConnectedBefore())
                    findObjectVector(wire).at(index - 1)->setConnectedAfter(0);
 
                 if(item->getConnectedAfter())
                    findObjectVector(wire).at(index +1 )->setConnectedBefore(0);
 
                 item->deleteLater();
                 findObjectVector(wire).remove(findObjectVector(wire).indexOf(item));
                 qDebug() << findObjectVector(wire).size();
             }
         }
     }
}

void QDSMainWindow::deleteWire(QDSWireObject *wire)
{
     if(!findObjectVector(wire).isEmpty())
     {
         Q_FOREACH(QDSWireObject* item, findObjectVector(wire))
         {
             if(item == wire)
             {
                 int index = findObjectVector(wire).indexOf(wire);

                 if(item->getConnectedBefore())
                    findObjectVector(wire).at(index - 1)->setConnectedAfter(0);

                 if(item->getConnectedAfter())
                    findObjectVector(wire).at(index +1 )->setConnectedBefore(0);

                 item->deleteLater();
                 findObjectVector(wire).remove(findObjectVector(wire).indexOf(item));
                 qDebug() << findObjectVector(wire).size();
             }
         }
     }
}

To copy to clipboard, switch view to plain text mode 

and I get the error C:\Documents and Settings\KY\Mina dokument\Dropbox\Qt Creator\DigitalSim\qmake\..\src\src\QDSMainWindow. cpp:204: error: passing 'const QVector<QDSWireObject*>' as 'this' argument of 'void QVector<T>::remove(int) [with T = QDSWireObject*]' discards qualifiers.

Why is this?

[wysota]

Because you are passing a const object to a non-const method. By the way... think whether calling findObjectVector() so many times in your deleteWire() method is really a good idea. I know CPU power is cheap nowadays, but is it really THAT cheap to execute the same O(n^2) method with the same argument a couple of times?

[meazza]

I know it is unnecessary to call it that many times but did this as a test to see if it would work instead of copying it.

---

Added after 5 minutes:

Look at line 3 of your code, you are making a copy of the vector there. You should be getting a warning from the compiler about returning a reference to a local object as well. By the way, your find method is not very optimal.

How could I optimize my find method?

[wysota]

Your whole approach is incorrect. Instead of trying to fix what you already have, throw it away and rewrite from scratch properly. Return indexes or pointers instead of references.

Here is what your findObjectVector() can look like:

int QDSMainWinwod::findObjectVector(QDSWireObject *wire) {
  for(int i=0;i<m_vectorVector.size();++i) {
    int index = m_vectorVector[i].indexOf(wire);
    if(index>=0) return i;
  }
  return -1;
}

int QDSMainWinwod::findObjectVector(QDSWireObject *wire) {
  for(int i=0;i<m_vectorVector.size();++i) {
    int index = m_vectorVector[i].indexOf(wire);
    if(index>=0) return i;
  }
  return -1;
}

To copy to clipboard, switch view to plain text mode 

It's still suboptimal because eventually you'll have to search through the vector again to find the index of the item, but it's a start... You can improve it like so:

int QDSMainWinwod::findObjectVector(QDSWireObject *wire, int *indexInVector = 0) {
  for(int i=0;i<m_vectorVector.size();++i) {
    int index = m_vectorVector[i].indexOf(wire);
    if(index>=0) {
        if(indexInVector) *indexInVector = index;
        return i;
    }
  }
  return -1;
}

int QDSMainWinwod::findObjectVector(QDSWireObject *wire, int *indexInVector = 0) {
  for(int i=0;i<m_vectorVector.size();++i) {
    int index = m_vectorVector[i].indexOf(wire);
    if(index>=0) {
        if(indexInVector) *indexInVector = index;
        return i;
    }
  }
  return -1;
}

To copy to clipboard, switch view to plain text mode 

and then:

void QDSMainWindow::deleteWire(QDSWireObject *wire) {
  int idx = -1;
  int vec = findObjectVector(wire, &idx);
  if(vec==-1) return;
  m_vectorVector[vec].removeAt(idx);
  delete wire;
}

void QDSMainWindow::deleteWire(QDSWireObject *wire) {
  int idx = -1;
  int vec = findObjectVector(wire, &idx);
  if(vec==-1) return;
  m_vectorVector[vec].removeAt(idx);
  delete wire;
}

To copy to clipboard, switch view to plain text mode