QMap insert QVector by pointer or value?

Question

I want to build up a map of devices such that the map contains:

QString 'DeviceID' and QVector 'Command List'

Currently I have the QMap as follows:

QMap<QString, QVector<QString> *> devices;

QVector<QString> *pCommands= new QVector<QString>;
//       :
// Fill pCommands with lots of data here
//       :
devices.insert(RadioID, pCommands);

But I am wondering if this is actually any better then:

QMap<QString, QVector<QString>> devices;

QVector<QString> commands;
//       :
// Fill commands with lots of data here
//       :
devices.insert(RadioID, commands);

I am sure that I read somewhere that Qt does something quite efficient when copying data. I am not seeing many people using pointers with Qt and it seems messy that I have to go through the QMap deleting all the QVector's at the end...

I am using c++11, so maybe some kind of move semantic may work here?

EDIT I have modified the comments in the code to show that the vector is not empty. Also I would state that I do not need to change the data once it is stored.

If you don't have a specific reason to use pointers then I would avoid them. — NathanOliver, Feb 01 '16 at 16:57
You are going to consume the memory regardless unless you are sharing them. If you are going to be "sharing" them then you should look at a shared pointer which will manage the pointer. — NathanOliver, Feb 01 '16 at 17:13
@NathanOliver It was not clear - but I meant for the vector to be full of data before I put it into the map. I have updated the question to make this clear : ) — code_fodder, Feb 01 '16 at 19:35
The whole reason for using containers is that they do the memory management for you. They are designed to store values in: the values of the type you're interested in, not the values of pointers. If the container is supposed to own the objects stored in it, then storing raw pointers in containers is always wrong, unless the container is designed to manage the data lifetime for you. None of the standard containers in C++ nor Qt do that - they are meant for storage of values, not pointers-to-values (unless they don't own the pointed-to-data). — Kuba hasn't forgotten Monica, Feb 02 '16 at 14:12

eerorika · Accepted Answer · 2016-02-01T19:43:18.997

4

There is no reason to consider manually allocating the vectors to be better.

Sure, you only need to copy a pointer, rather than a vector, but an empty vector is still quite fast to copy. The biggest gain of storing objects rather than pointers is that you don't need to do manual memory management.

I am using c++11, so maybe some kind of move semantic may work here?

If QMap::insert supports move semantics, and if QVector is move-constructible like their standard library counterparts, then you could indeed move the vector into the map. But moving an empty vector is just as fast as copying.

If QMap has an emplace like function std::map does, then you could even construct the vector in-place without even a move.

I'm not familiar with Qt, though so you'll need to verify those details from the documentation. Whether Qt supports move semantics doesn't change the fact that manual memory management is a pain.

Edit: according to the documentation QVector appears to be movable, but QMap does not support move semantics. However, as Arpegius and the documentation point out, QVector does copy-on-write optimization, so as long as the copied vector is not modified, then the data won't be copied. None of this matters really, when copying an empty vector.

Edit again

If the added vectors are full of data, then copying is indeed quite expensive unless it remains unmodified. Moving would remedy that, but QMap appears not to support that. There is another trick, though: Insert an empty vector, and then swap the full vector with the empty one in the map.

edited Feb 01 '16 at 19:43

answered Feb 01 '16 at 17:13

eerorika

232,697
12
197
326

@code_fodder Ufortunatly `QMap::insert` do not use move semantics, but as http://doc.qt.io/qt-5/qvector.html#QVector-3 its avoiding the copy of whole QVector, so the move semantics is not needed here. – Arpegius Feb 01 '16 at 17:16
@Arpegius well, move semantics would still be needed, when the copied vector needs to be modified - which is something I would assume code_fodder needs to do - because that would trigger copying of the data. But there's no need for move, nor COW if you copy an empty vector like code_fodder is doing :) – eerorika Feb 01 '16 at 17:24
Right, I didn't assume that he is always inserting empty vector. – Arpegius Feb 01 '16 at 17:30
Thanks very much for the information. I got my question slightly wrong, in that it was meant to be assumed that the QVector would be full of data - but I did not copy that across. Does this change things? I have (or will have in a minute) edited my question ... – code_fodder Feb 01 '16 at 19:32
@code_fodder edited again. You can use `swap` to implement move without move semantics. – eerorika Feb 01 '16 at 19:47
Thanks, actually... apart from the code looking a bit odd (nothing a comment can't fix) this looks a good solution. The docs say: "This operation is very fast and never fails." which ticks my other box of speed efficiency - but that is secondary for now :) +1 answer :) – code_fodder Feb 01 '16 at 20:18

Kuba hasn't forgotten Monica · Answer 2 · 2016-02-02T14:27:32.707

1

The simplest and pretty much idiomatic way to do it would be:

QMap<QString, QVector<QString>> devices;

// Get the vector constructed in the map itself.
auto & commands = devices[RadioID];
// Fill commands with lots of data here - a bit faster
commands.resize(2);
commands[0] = "Foo";
commands[1] = "Bar";
// Fill commands with lots of data here - a bit slower
commands.append("Baz");
commands.append("Fan");

You'll see this pattern often in Qt and other code, and it's the simplest way to do it that works equally well on C++98 and C++11 :)

This way you're working on the vector that's already in the map. The vector itself is never copied. The members of the vector are copied, but they are implicitly shared, so copying is literally a pointer copy and an atomic reference count increment.

edited Feb 02 '16 at 14:27

answered Feb 02 '16 at 14:18

Kuba hasn't forgotten Monica

95,931
16
151
313

Thanks for that Kuba (+1), if I recall correctly the reference count increment is a Qt specific behaviour so this solution is specific to Qt containers (i.e. does not work as well for c++ STL)? – code_fodder Feb 02 '16 at 20:53
@code_fodder The only performance difference here would be between `QString` and `std::string`. The `commands` is a reference to a vector already in the map. You could use `std::map>` and it would perform exactly the same as the above. If you don't wish to copy the strings, use `emplace_back` instead of `append`. – Kuba hasn't forgotten Monica Feb 03 '16 at 13:37
Can we be sure that changes to the map won't invalidate the reference? – Thomas Sep 01 '16 at 10:02
You're not supposed to use the reference past any map modifications, of course. Treat it like you would an iterator. But you're free to modify the map element that the reference refers to, of course. – Kuba hasn't forgotten Monica Sep 01 '16 at 13:11

QMap insert QVector by pointer or value?

2 Answers2