QMap iterator Bug??

IknowQT

QMap<QString, CLogin::AccountInfo>::iterator iter;
for (iter = m_pLogin->getAccountMgr().begin(); iter != m_pLogin->getAccountMgr().end(); ++iter)
{
	UpdateTableWidget(iter);
}

void wLoginMgr::UpdateTableWidget(QMap<QString, CLogin::AccountInfo>::iterator iter)
{
	int nIndex = this->ui.tableWidget->rowCount();
	this->ui.tableWidget->insertRow(nIndex);

	QTableWidgetItem* itemID	= new QTableWidgetItem(iter.key());
	QTableWidgetItem* itemAuth	= new QTableWidgetItem(m_pLogin->EnumtoQString(iter.value().eAuthority));
	QTableWidgetItem* itemDate	= new QTableWidgetItem(iter.value().createDate.toString(m_pLogin->getDateFormat()));
	itemID->setTextAlignment(Qt::AlignCenter);
	itemAuth->setTextAlignment(Qt::AlignCenter);
	itemDate->setTextAlignment(Qt::AlignCenter);

	this->ui.tableWidget->setItem(nIndex, 0, itemID);
	this->ui.tableWidget->setItem(nIndex, 1, itemAuth);
	this->ui.tableWidget->setItem(nIndex, 2, itemDate);
}

I'm writing an example that puts items in a tableWidget using qmap iterator, but I can't access it with iter, so I ask a question.
Exception handling occurs the moment iter is accessed.

If I use Qmapiterator inside the m_pLogin class, it works as well.
It doesn't seem to work when accessing from the outside. Is there a workaround:?

Christian Ehrlicher

C++ stuff

m_pLogin->getAccountMgr().begin()
m_pLogin->getAccountMgr().end()

You're creating two temporary container here which are out-of-scope so your program will crash.

IknowQT

@Christian-Ehrlicher

I don't quite understand, but usually iterate isn't used the way I wrote it? Should ++ increase from begin?

Christian Ehrlicher

@IknowQT said in QMap iterator Bug??:

Should ++ increase from begin?

Don't know what this has to do with my comment above.

m_pLogin->getAccountMgr().begin()

This creates a temporary container and calls begin on it, then the container is destroyed and the iterator is dangling.
Create a local copy of the container you want to iterate over.

IknowQT

@Christian-Ehrlicher

QMap<QString, CLogin::AccountInfo>::iterator iter = m_pLogin->getAccountMgr().begin();
			for (iter; iter != m_pLogin->getAccountMgr().end(); iter++)
			{
				UpdateTableWidget(iter);
			}

Does it mean I should do it like this?

Christian Ehrlicher

@IknowQT said in QMap iterator Bug??:

Does it mean I should do it like this?

No, since you still create two temporary container

m_pLogin->getAccountMgr()...

IknowQT

@Christian-Ehrlicher

Can you tell me in detail what to do?
Can you tell me if you have an example code?

Christian Ehrlicher

@IknowQT said in QMap iterator Bug??:

Can you tell me in detail what to do?

I already did:

Create a local copy of the container you want to iterate over.

Can you tell me if you have an example code?

Maybe a c++ book would be a good start

/edit: it's even described in the Qt documentation: https://doc.qt.io/qt-5/containers.html (Search for '// RIGHT' and '// WRONG')

IknowQT

@Christian-Ehrlicher

QMap<QString, CLogin::AccountInfo> test = m_pLogin->getAccountMgr();
			QMap<QString, CLogin::AccountInfo>::iterator iter;// = test.begin();
			for (iter = test.begin(); iter != test.end(); ++iter)
			{
				UpdateTableWidget(iter);
			}

Are you telling me to write it like this?
I tested it and it worked well.

KroMignon

@IknowQT said in QMap iterator Bug??:

Are you telling me to write it like this?

What @Christian-Ehrlicher is telling you, is that you are creating 2 different container.
Each time you are calling m_pLogin->getAccountMgr(), it will return a QMap instance.
So each one has a different begin and end.
So you are comparing things which cannot be compared.

But yes, this is the right way to do, as it is given in Qt documentation:

// store local copy of the map
auto &test = m_pLogin->getAccountMgr();
// iterate through the map
for (auto iter = test.begin(); iter != test.end(); ++iter)
{
    UpdateTableWidget(iter);
}

Christian Ehrlicher

@KroMignon said in QMap iterator Bug??:

So each one has a different begin and end.

No, it's even worse - as I said above the two iterators are dangling because the returned containers are temporaries.

IknowQT

@Christian-Ehrlicher

I think this topic is endless. Can you post some concrete example code?

jsulm

@IknowQT said in QMap iterator Bug??:

I think this topic is endless. Can you post some concrete example code?

You already have it and you even wrote that it is working...

KroMignon

@IknowQT said in QMap iterator Bug??:

I think this topic is endless. Can you post some concrete example code?

Why endless? All has been explained.
It is not a QMap iterator bug. It is your code which is wrong.
Each time you are calling m_pLogin->getAccountMgr() is returns a new QMap instance.
Your code has many errors:

first : in the initialization statement in the for loop you are setting up iter is a begin of the return QMap.
second: this return QMap is temporary, which means it is destroyed directly after usage ==> iter becomes a dangling pointer!!!
third: in the condition statement, comparing iter with another QMap instance (which also is temporary!!)

Solution for this ==> learn C++ basics like variable/objects lifetime! (cf. https://en.cppreference.com/w/cpp/language/lifetime)

JoeCFD

@KroMignon if m_pLogin->getAccountMgr() returns const Whatever &, his code will work as well. But it is not great to code like he does. I think the following loop for map may be simpler

    std::map<std::string, int > test_map;
    test_map[ "test1" ] = 1;
    test_map[ "test2" ] = 2;
    for ( const std::pair< std::string, int > & p : test_map ) {
        std::cout << p.first << '\t' << p.second << std::endl;
    }

KroMignon

@JoeCFD said in QMap iterator Bug??:

if m_pLogin->getAccountMgr() returns const Whatever &, his code will work as well.

Yes, but that was not the point (I guess)

I think the following loop for map may be simpler

To be honest, there is no real difference. You are using std::map instead of QMap, that's all.
It is only a question of taste... Preferring std::map over QMap.
There is no need to use QMap::iterator to iterate through a QMap, it is only a possibility ;)

JoeCFD

@KroMignon not only a taste issue. I believe std containers may be better optimized than Qt containers in most cases.

Christian Ehrlicher

@JoeCFD said in QMap iterator Bug??:

I believe

... so nonsense without any real background.
And the above usecase which the topic is about (and your posts simply ignore) would be much slower with std::map than QMap since the contents of the std::map would have been copied.

KroMignon

@JoeCFD said in QMap iterator Bug??:

I believe std containers may be better optimized than Qt containers in most cases.

Not according to this benchmark done by woboq => https://woboq.com/blog/qmap_qhash_benchmark.html
or this stackoverflow entry => https://stackoverflow.com/questions/1668259/stl-or-qt-containers

JoeCFD

@KroMignon said in QMap iterator Bug??:

@JoeCFD said in QMap iterator Bug??:

I believe std containers may be better optimized than Qt containers in most cases.

Not according to this benchmark done by woboq => https://woboq.com/blog/qmap_qhash_benchmark.html <=== done in 2012 ages old.
or this stackoverflow entry => https://stackoverflow.com/questions/1668259/stl-or-qt-containers read comments: 35