QMap iterator Bug??

IknowQT

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

Christian Ehrlicher

@JoeCFD When you're at the point where you really need to worry about a speed difference between those implementations you should write your own implementation of the problematic container. You will never notice if a running program is using c++ or Qt or boost or whatever containers.