CODEWITHSUNDEEP
objective-cioscocoa-touch
Padin215
Padin215

Reputation: 7484

why did fast enumeration work when standard for loop failed?

I have a UIView *buttonView filled with buttons. I need to update my list and re-populate my buttonView. I implemented:

if ([[self.buttonView.subviews objectAtIndex:i] isKindOfClass:[UIButon class]]
    [[self.buttonView.subviews objectAtIndex:i] removeFromSuperview];

but it failed, it would not remove all the buttons (with my test with 8 buttons, it removed every other button :-?)

I then tried: 

for(UIView *subview in self.buttonView.subviews) 
{
    if([subview isKindOfClass:[UIButton class]]) 
        [subview removeFromSuperview];
}

and it worked perfectly.

Shouldn't both loops accomplish the same thing?

I'm guessing there is something I don't know about fast enumeration that would explain this?

Upvotes: 1

Views: 287

Answers (4)

Joel Kravets
Joel Kravets

Reputation: 2471

Just like everybody else said your main problem is modifying a mutable array while iterating it. Instead you can find all the buttons that are subviews of your buttonView and make them perform removeFromSuperview.

id buttonClassTest = ^(UIView * subview, NSUInteger idx, BOOL *stop){
    return [subview isKindOfClass:[UIButton class]];
};

NSIndexSet * indexes = [self.buttonView.subviews indexesOfObjectsPassingTest:buttonClassTest];
NSArray * buttons = [self.buttonView.subviews objectsAtIndexes:indexes];
[buttons makeObjectsPerformSelector:@selector(removeFromSuperview)];

Upvotes: 0

David Kanarek
David Kanarek

Reputation: 12613

I don't believe you're allowed to modify the collection you're enumerating, so be careful with that. It seems like it handles the changes just fine though, and it will get through every item (unlike the for loop).

In the first case, it wouldn't work because once you remove index 0, something else becomes index 0, but you've moved on to index 1. You could fix it by decrementing your loop variable in the event that the remove succeeds, so that the loop increments it to its previous value:

if ([[self.buttonView.subviews objectAtIndex:i] isKindOfClass:[UIButton class]]) {
    [[self.buttonView.subviews objectAtIndex:i] removeFromSuperview];
    --i;
}

What happens with your for loop:

1,2,3,4,5,6,7,8 and remove first item (1)

2,3,4,5,6,7,8 and remove second item (3)

2,4,5,6,7,8 and remove third item (5)

2,4,6,7,8 and remove fourth item (7).

2,4,6,8 and you're on index 5, which is greater than length, so you're done.

Upvotes: 2

Lily Ballard
Lily Ballard

Reputation: 185671

Your fundamental problem is you're mutating a collection while iterating. If you really need to do this, you may want to consider iterating backwards (with a -reverseObjectEnumerator or with an -enumerateObjectsWithOptions:usingBlock: with the reverse option). Alternatively you can create a copy of the array before iterating over it.

The reason fast enumeration is working is likely due to the call to -subviews returning some sort of constant copy of the subview array. This could actually be used in your first case just by calling -subviews once and re-using the result, but this would introduce a dependency upon an implementation detail and I would strongly recommend avoiding it. If you need a constant copy of the array, you should make one yourself.

The quickest solution to fix case 1 would be:

NSArray *subviews = [NSArray arrayWithArray:self.buttonView.subviews];
// now enumerate using the subviews array directly instead of calling self.buttonView.subviews

For your fast-enumeration case, you may want to rewrite that as

for (UIView *subview in [NSArray arrayWithArray:self.buttonView.subviews]) {
    // ...
}

A third option is switching to the block-based enumeration and going in reverse. This is actually going to be the most efficient choice if it's one you can make:

[self.buttonView.subviews enumerateObjectsWithOptions:NSEnumerationReverse usingBlock:^(UIView *subview, NSUInteger idx, BOOL *stop) {
    if ([subview isKindOfClass:[UIButton class]]) {
        [subview removeFromSuperview];
    }
}];

Upvotes: 5

mpontillo
mpontillo

Reputation: 13947

Your for loop would have worked if you ran it in reverse (start at the highest index and loop while i > 0). The way the for loop is written, every time you remove an object, the indexes change. The way the enumeration is written, it's either safe to remove objects mid-enumeration or all the objects are returned up-front, which wouldn't leave any opportunity to skip any elements.

Upvotes: 3

Related Questions