On 07.04.2013 09:06, pulkomandy@xxxxxxxxxxxxx wrote:
void TExpandoMenuBar::RemoveTeam(team_id team, bool partial) { - int32 count = CountItems(); - for (int32 i = 0; i < count; i++) { + TWindowMenuItem* windowItem = NULL; + + for (int32 i = 0; i < CountItems(); i++) {This is counting the items at every iteration, which wastes CPU. It seems you are changing the item count inside the loop, which I guess is why you did that. There are at least two ways to do that : // Looping backwards, removing an item does not change the part of the list // you still have to browse for (int32 i = CountItems(); --i >= 0 ;) { }
When removing items while iterating a list, I think this form above is the way to go. However, I don't like the obfuscated syntax so much, I prefer:
for (int32 i = CountItems() - 1; i >= 0; i--) { ... }I already deleted the original mail and can't check, but just noting that even when you account for the changed item count in the loop, you still need to decrement i in order not to skip items in the iteration when going forward. The backwards loop is really the cleanest way to do it.
Best regards, -Stephan