[Marble-devel] Fwd: Re: Wrong usage of ?QList::ConstIterator & QList::begin() ?

[Torsten Rahn]

On Saturday 18 October 2008 15:38:00 Inge Wallin wrote:
> This struck me as something we should check in Marble.  Not only for bug
> reasons but also performance.

Yeah, 

I had a quick look already this afternoon by doing a grep but found only a 
single occurrence in GeoPolygon (which neither seemed harmful nor performance 
relevant however should be fixed.

Inge, could you have a closer look and fix this where needed?
I think it would also be nice if we'd just use one kind of ConstIterator 
keyword (currently we use const_iterator as well as ConstIterator) for
consistency :-)

Regards,

Torsten

> 	-Inge
>
> ----------  Forwarded Message  ----------
>
> Subject: Re: Wrong usage of ?QList::ConstIterator & QList::begin() ?
> Date: Saturday 18 October 2008
> From: Andreas Pakulat <apaku at gmx.de>
> To: kde-core-devel at kde.org
>
> On 18.10.08 12:40:34, Albert Astals Cid wrote:
> > A Dissabte 18 Octubre 2008, Christian Ehrlicher va escriure:
> > > Hi,
> > >
> > > While trying to fix some bugs in KFile* on windows I saw some places
> > > like this:
> > >
> > > for ( QStringList::ConstIterator it = m_dirList.begin();
> > >        it != m_dirList.end() && !terminationRequested();
> > >        ++it )
> > >
> > > Imo the programmer here expects that m_dirList.detach() isn't called
> > > but this is wrong.
> > >
> > > m_dirList.begin() does a detach, returns a non-const iterator and then
> > > the iterator is converted to a const iterator (see qlist.h:214 & 238).
> > >
> > > Therefore I suggest to add QT_STRICT_ITERATORS to the kdelibs flags. Or
> > > does this maybe break BC?
> >
> > This is not wrong, just not what the developer "really wanted" but it
> > works just fine, albeit a bit slower.
>
> Actually it is wrong, or rather it can go wrong. Particularly the it !=
> m_dirList.end() might evaluate to true on the first test, so the loop
> wouldn't be executed at all. Its not so long ago when I fixed a bug
> somewhere (not sure it was in KDE) that suffered exactly from that kind of
> thing. If you want a const iterator use constBegin+constEnd.
>
> Andreas