aacid added a comment.
In https://phabricator.kde.org/D4439#83326, @dfaure wrote: > In https://phabricator.kde.org/D4439#83310, @aacid wrote: > > > In https://phabricator.kde.org/D4439#83166, @dfaure wrote: > > > > > It's not crazy, but > > > > > > - then it should use QVector instead of QList (Client is a "big" struct, bigger than a pointer) > > > > > > The problem with QVector is that it doesn't have erase(iterator) built in like QList has. > > > I see it in the docu: > > iterator QVector::erase(iterator pos) > Removes the item pointed to by the iterator pos from the vector, and returns an iterator to the next item in the vector (which may be end()). > > >> - I would be worried about copies happening unexpectedly (can this code compile with forbidden copy ctor for Client? I guess not as is due to insertion into the vector.... but maybe std::move can be used there, or simply setting the members directly onto a ref for vector[i]). > > > > Without the copy constructor there's quite a lot of things that don't work. OTOH all the data in Client is basiclaly POD, but i guess at some point it could be "a lot of copying", if you think it's worth it i can investigate some "less Q and more C++11-y stuff" and see if std::move or something works > > If you fancy looking into it, go ahead. Otherwise tell me and I will. You take it, had a look and it's not as easy as i'd like, m_mapEntries makes it more difficult, and also the code kind of confuses me by m_entries containing pointers that seem to be extracted from the map itself? Maybe let's not touch this and just go with your simpler solution :D REPOSITORY R244 KCoreAddons BRANCH master REVISION DETAIL https://phabricator.kde.org/D4439 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: dfaure, mpyne, aacid Cc: markg, #frameworks