[haiku-commits] Re: haiku: hrev45452 - src/apps/deskbar

  • From: pulkomandy@xxxxxxxxxxxxx
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Sun, 07 Apr 2013 09:06:42 +0200

>  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 ;)
{

}

// Decrementing the total count when removing an item
int32 itemCount = CountItems();
for(int32 i = 0; i < itemCount;)
{
        if(needsToRemove)
        {
                Remove(i);
                itemCount--;
                // run the next loop iteration with the same index
        } else {
                i++;
        }
}

However, you are exiting from the loop after removing, so all this is not 
needed and you could keep the CountItems in a separate variable (make it 
const) and use a regular for.

A call to CountItems in a loop can't be optimized by the compiler because the 
result may be different at each loop iteration. The method is called 
CountItems which implies it is actually counting. Otherwise we would call it 
GetItemCount, and it would be ok to call it at each iteration.

-- 
Adrien.

Other related posts: