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

  • From: Stephan Aßmus <superstippi@xxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Sun, 07 Apr 2013 14:02:41 +0200

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


Other related posts: