[Kde-games-devel] kspaceduel: replace Q3PtrList with QtList

[Albert Astals Cid]

A Dijous 04 Gener 2007 22:24, Dmitry Suzdalev va escriure:
> On Thursday 04 January 2007 23:52, Dirk wrote:
> > I tried to remove every Q3PtrList from kspaceduel to finally finish
> > porting it to Qt4. I replaced Q3PtrList with QList<*> and used
> > index-based QLists as recommended by the Qt docs. The resulting patch is
> > attached. However, I'm not sure whether it's correct and it would be
> > nice if someone could take a short look at it, because if I did
> > something wrong it would be hard to find the mistake later when it
> > causes trouble.
>
> Hi, Dirk!
>
> I looked at your patch and:
> 1. I believe that removing items from container while looping over it can
> lead to unexpected results. IIRC QLinkedList is the container to use if you
> want to have iterators which aren't invalidated after remove* operations.
> Please check this in doc :).
> 2. Again, you can use foreach() instead "for(int i=0;i<list.size();i++)'
> kind of loops to make the code more readable.
> 3. Why you use qDeleteAll() in one places and "while(!empty) takeFirst" in
> another? I think it would be nice to be consistent.
>
> Well, 2,3 are cosmetic issues, while number 1 can cause some problems if
> I'm not mistaken :).

No, it is only problematic if you keep iterators as in QList they are not 
guaranteed to remain correct once you remove any element, but he is accessing 
with [] and checking i with size so it should not have any problem.

Albert

>
> These Q3PtrLists->QLists transition can be not that easy, I know :-).
> Nice work!
>
> Cheers,
> Dmitry.
> _______________________________________________
> kde-games-devel mailing list
> kde-games-devel at kde.org
> https://mail.kde.org/mailman/listinfo/kde-games-devel