Problem using QList::RemoveAt function

[Indrajeet]

Hi

In have created a QList<MyObjects*> mylist;
mylist contains 10 MyObjects*.

Now i want to remove one by one object so i was doing some thing like this but it is crashing

@
int itotal = mylist.count();
for(int i =0 ; i< itotal ; i++)
{
if(condition)
mylist.removeAt(i);
}
@
As i am removing each time the size is getting reduced but the for loop is running for total number of count and it is crashing.
Can anyone tell me the trick so that i can delete the paricular object which satisfies the condition and also i should be able to
traverse for full list count.

[koahnig]

Probably the simplist trick would be to go from the back

@
int itotal = mylist.count();
for(int i = itotal; i > 0; --i)
{
if(condition)
mylist.removeAt(i-1);
}
@

That would be my first check to get it work. I do not work with QList, but there might be better ways to do.

[lgeyer]

@
int i = 0;
while(i < mylist.count())
{
if(condition)
mylist.removeAt(i);
else
i++;
}
@

[lgeyer]

[quote author="koahnig" date="1327568122"]
@
int itotal = mylist.count();
for(int i = itotal; i > 0; --i)
{
if(condition)
mylist.removeAt(i-1);
}
@
[/quote]
-The number of items in the list decreases if there is at least one item matching the condition and indices change when items are removed thus fixed loop-tests and fixed counting expressions are excluded, disqualifiying couting down <code>for</code>.-

[KA51O]

I use this for complete destruction:

@
while(!list.isEmpty())
{
delete list.takeFirst();
}
@

a shot at a solution with condition (not guaranteed to work):
@
int size = list.count();
for(int i = 0; i < size; ++i)
{
if(condition)
{
YourType* objToDelete = list.at(i);
list.removeAt(i);
i--;
size--;
delete objToDelete;
}
}
@

[koahnig]

[quote author="Lukas Geyer" date="1327568456"]
The number of items in the list decreases if there is at least one item matching the condition and indices change when items are removed - thus fixed loop-tests and fixed counting expressions are excluded, disqualifiying <code>for</code>.[/quote]

I admit it is not the most elegant solution and I would vote for your while loop approach. Nevertheless, I do not agree that there be a problem as long as nobodyelse is fidling with the counter i.

[koahnig]

[quote author="KA51O" date="1327570403"]I use this:

@
while(!list.isEmpty())
{
delete list.takeFirst();
}
@
[/quote]

The condition is missing in your case and
@
list.clear();
@

would be shorter and probably faster then.

[KA51O]

[quote author="koahnig" date="1327571283"][quote author="KA51O" date="1327570403"]I use this:

@
while(!list.isEmpty())
{
delete list.takeFirst();
}
@
[/quote]

The condition is missing in your case and
@
list.clear();
@

would be shorter and probably faster then. [/quote]

actually list.clear() only removes the objects from the list. It does not delete the objects, which of course is fine if you still use them somewhere else.

I was still editing the post to add a solution with a condition ^^

[koahnig]

[quote author="KA51O" date="1327571471"][quote author="koahnig" date="1327571283"][quote author="KA51O" date="1327570403"]I use this:

@
while(!list.isEmpty())
{
delete list.takeFirst();
}
@
[/quote]

The condition is missing in your case and
@
list.clear();
@

would be shorter and probably faster then. [/quote]

actually list.clear() only removes the objects from the list. It does not delete the objects, which of course is fine if you still use them somewhere else.

[/quote]

Sorry, I did not read carefully enough :S I see now.

Your approach would mean that you store pointers in the list. You delete the objects then, but this would be an endless loop.

[KA51O]

[quote author="koahnig" date="1327571719"][quote author="KA51O" date="1327571471"][quote author="koahnig" date="1327571283"][quote author="KA51O" date="1327570403"]I use this:

@
while(!list.isEmpty())
{
delete list.takeFirst();
}
@
[/quote]

The condition is missing in your case and
@
list.clear();
@

would be shorter and probably faster then. [/quote]

actually list.clear() only removes the objects from the list. It does not delete the objects, which of course is fine if you still use them somewhere else.

[/quote]

Sorry, I did not read carefully enough :S I see now.

Your approach would mean that you store pointers in the list. You delete the objects then, but this would be an endless loop. [/quote]

No because QList::takeFirst() removes the first item and returns it at the same time.

[KA51O]

this is Lukas Geyers solution with delete:

@
int i = 0;
while(i < mylist.count())
{
if(condition)
{
YourType* objToDelete = mylist.at(i);
mylist.removeAt(i);
delete objToDelete;
}
else
i++;
}

@

[koahnig]

[quote author="KA51O" date="1327571898"]
No because QList::takeFirst() removes the first item and returns it at the same time.
[/quote]
Correct. Not my day :-(

[lgeyer]

[quote author="koahnig" date="1327571110"]I admit it is not the most elegant solution and I would vote for your while loop approach. Nevertheless, I do not agree that there be a problem as long as nobodyelse is fidling with the counter i.[/quote]
Well, it looks like I agree with your disagreement (at least as long as it comes to counting down <code>for</code>) ;-)

[Indrajeet]

Thanks Lukas While loop works correctly.

[goetz]

[quote author="koahnig" date="1327568122"]Probably the simplist trick would be to go from the back

@
int itotal = mylist.count();
for(int i = itotal; i > 0; --i)
{
if(condition)
mylist.removeAt(i-1);
}
@

That would be my first check to get it work. I do not work with QList, but there might be better ways to do.

[/quote]

I regard the above solution as the most elegant and easy to read one, although I would modify and shorten it a bit more:

@
for(int i = mylist.count() - 1; i >= 0; --i)
{
if(condition_is_met)
delete mylist.takeAt(i);
}
@

[Indrajeet]

So while loop is best approach is it?

[goetz]

[quote author="Rajveer" date="1327573850"]So while loop is best approach is it?[/quote]

Use the solution that works for you and that you understand how it works.

[_Michel]

@
int i = 0;
// size() and count() are identical unless you give a paremeter to count(const T & value)
while (i < myList.size())
{
if (condition is met)
{
myList.removeAt(i); // "If you don't use the return value, removeAt() is more efficient.":http://qt-project.org/doc/qt-5/qlist.html#takeAt
}
else
{
i++;
}
}
@

[KA51O]

But if you need to call delete then you need to use takeAt. Which is what was discussed above.
Anyways your code is just like the one posted by Lukas Geyer in the 3rd post.

[quote author="Lukas Geyer" date="1327568215"]@
int i = 0;
while(i < mylist.count())
{
if(condition)
mylist.removeAt(i);
else
i++;
}
@[/quote]