Containers with C++17 structured bindings

[jsulm]

@david_dlqa said in Containers with C++17 structured bindings:

Is there a plan to add this in and/or a way to make it work in a similar way?

Hi and welcome!

This is more a question for Qt developers (on their mailing list). Here we are in a user forum.

[deleted541]

Hi, thanks for letting me know.
I guess the question then is, if plain iterators are the best way to do this if I need both the key and value in a loop.

[Chris Kawa]

Unlike std::map Qt does not store the key/value in a pair, so you can't bind your structure to the result of operator*. It returns only the value.

If you need both key and a value you can either use the map's iterator:

for( auto it = qt_m.cbegin(); it != qt_m.cend(); ++it )
{
    qDebug() << it.key() << " : " << it.value() << '\n';
}

or the iterator class:

QMapIterator it(qt_m);
while(it.hasNext())
{
    it.next();
    qDebug() << it.key() << " : " << it.value() << '\n';
}

[fcarney]

Or:

for(auto key: qt_m.keys()){
  qInfo() << key << qt_m[key];
}

[Chris Kawa]

@fcarney Yeah, but that's a very bad solution. First keys() makes a copy of the data into a new container and then you make a find for each element (that's what operator[] is essentially). It's terrible both performance wise and for memory consumption. Please don't do this in production code.

[fcarney]

@Chris-Kawa said in Containers with C++17 structured bindings:

It's terrible both performance wise and for memory consumption.

Now I can see why keys() is not in the std::map. It would encourage poor coding practices. Thanks for your insight. I generally use this to see whats in my map for debugging, but yeah I can see the performance problems it would create on large datasets.

[SGaist]

Hi,

Just in case, there are key/value iterators for both QMap and QHash:
QMap::keyValueBegin
QMap::keyValueEnd
QMap::constKeyValueBegin
QMap::constKeyValueEnd
QHash::keyValueBegin
QHash::keyValueEnd
QHash::constKeyValueBegin
QHash::constKeyValueEnd

[Chris Kawa]

@SGaist Huh, that's cool. I didn't know those existed. Thanks!

@david_dlqa So you could write a little adapter thingy like this:

template<class Container> class adapt
{
public:
    adapt(const Container& container) : c(container) {}
    typename Container::const_key_value_iterator begin() const { return c.keyValueBegin(); }
    typename Container::const_key_value_iterator end() const { return c.keyValueEnd(); }
private:
    const Container& c;
};

and then you could use structured bindings like so:

for( const auto& [key, value] : adapt(qt_m) )
{
    qDebug() << key << " : " << value << '\n';
}

From what i see it's still an inferior solution performance wise, as those keyValue iterators make pairs on the fly, but it might be good enough.

[kshegunov]

@fcarney said in Containers with C++17 structured bindings:

I generally use this to see whats in my map for debugging, but yeah I can see the performance problems it would create on large datasets.

You don't want to keep large datasets in a map to begin with. [1]

[1]: https://woboq.com/blog/qmap_qhash_benchmark.html

[deleted541]

@Chris-Kawa
Nice, thats very helpful. I think the performance penalty here is negligible. I ran a test on my machine which produced these results in release mode:

const iter 2799
non-const iter 2835
adapt const-reference 2819
adapt copy 2808

Test code:

#include <QMap>
#include <map>
#include <QDebug>
#include <QElapsedTimer>

template<class Container> class adapt
{
public:
	adapt(const Container& container) : c(container) {}
	typename Container::const_key_value_iterator begin() const { return c.keyValueBegin(); }
	typename Container::const_key_value_iterator end() const { return c.keyValueEnd(); }
private:
	const Container& c;
};

int main()
{
	QMap<int, QString> qt_m;
	for(int i = 0; i < (1<<24); ++i) qt_m.insert(rand(), QString::number(rand()));
	QString a;
	int b;

	QElapsedTimer t;
	t.start();
	for( auto it = qt_m.cbegin(); it != qt_m.cend(); ++it )
	{
		a = *it;
		b = it.key();
	}
	qDebug() << "const iter" << t.elapsed();

	t.restart();
	for( auto it = qt_m.begin(); it != qt_m.end(); ++it )
	{
		a = *it;
		b = it.key();
	}
	qDebug() << "non-const iter" << t.elapsed();

	t.restart();
	for( const auto& [key, value] : adapt(qt_m) )
	{
		a = value;
		b = key;
	}
	qDebug() << "adapt const-reference" << t.elapsed();

	t.restart();
	for( auto [key, value] : adapt(qt_m) )
	{
		a = value;
		b = key;
	}
	qDebug() << "adapt copy" << t.elapsed();
 }

[J.Hilk]

@david_dlqa said
Just a word of caution, as I ran into this as well.

If you do speed tests with QElapsedTimer, do not simply print the elapsed() value to qDebug().

qDebug() << "const iter" << t.elapsed();

the qDebug() call will take a significant amount of time.

I usually store the elapsed value in a local variable and print that one at time insensitive points (the end)

[deleted541]

@J.Hilk
Thanks, that makes sense. I just copied that from the documentation hence I thought that was not a concern.
I reran the tests and got almost the same results, so I think with times in the seconds, the qDebug overhead is not significant