Array of pointers causes leaks

Question

Hi I have this code

-(void)setUserFilters{

    //init the user filters array
    userFilters = [[NSMutableArray alloc] init];
    SearchCriteria *tmpSc= [[SearchCriteria alloc] init];
    for(int i=0;i<[searchFilters count];i++)
    {
 tmpSc=[self.searchFilters objectAtIndex:i];
 if(tmpSc.enabled==TRUE)
  [userFilters addObject:tmpSc];
    }
    }

searchFilters is a list of filters that can be setted to true or false and I use userFilters to populate a table view with the filters that are only setted to TRUE

But the line SearchCriteria *tmpSc= [[SearchCriteria alloc] init]; causes leaks, and I don't know how to solve because if I release at the end of the function I loose my pointers and it crashes

Any ideas?

Answer 1

first of all, the worst n00b code you can write involves if(condition==true) do_something(), just write if(condition) do_something().

second, there is no reason to have tempSc at all (never mind alloc memory for it), you can just do the following:

-(void)setUserFilters{

//init the user filters array
userFilters = [[NSMutableArray alloc] init];
for(int i=0;i<[searchFilters count];i++)
{
    if([self.searchFilters objectAtIndex:i].enabled)
            [userFilters addObject:[self.searchFilters objectAtIndex:i]];
}
}

Answer 2

Hi Mathieu,

It seems that your initially creating a SearchCriteria object and before you use it or release it your reassigning the variable to another object from self.searchFilters. So you don't need to create the initial object and why it's leaking and not being released.

Try:

SearchCriteria *tmpSc= null;

Hope that helps.

Answer 3

twolfe18 has made the code >much slower if searchFilters can be large. -objectAtIndex: is not a fast operation on large arrays, so you shouldn't do it more than you have to. (While true that FE is faster than objectAtIndex:, this overstated the issue and so I've striken it; see my other comments on the advantages of Fast Enumeration.)

There are a number of problems in your code:

• Never create a method that begins "set" but is not an accessor. This can lead to very surprising bugs because of how Objective-C provides Key-Value Compliance. Names matter. A property named userFilters should have a getter called -userFilters and a setter called -setUserFilters:. The setter should take the same type that the getter returns. So this method is better called -updateUserFilters to avoid this issue (and to more correctly indicate what it does).

• Always use accessors. They will save you all kinds of memory management problems. Your current code will leak the entire array if -setUserFilters is called twice.

• Both comments are correct that you don't need to allocate a temporary here. In fact, your best solution is to use Fast Enumeration, which is both very fast and very memory efficient (and the easiest to code).

Pulling it all together, here's what you want to be doing (at least one way to do it, there are many other good solutions, but this one is very simple to understand):

@interface MyObject ()
@property (nonatomic, readwrite, retain) NSMutableArray *userFilters;
@property (nonatomic, readwrite, retain) NSMutableArray *searchFilters;
@end

@implementation MyObject
@synthesize userFilters;
@synthesize searchFilters;

- (void)dealloc
{
    [searchFilters release];
    serachFilters = nil;
    [userFilters release];
    userFilters = nil;
    [super dealloc];
}


- (void)updateUserFilters
{
    //init the user filters array
    // The accessor will retain for us and will release the old value if
    // we're called a second time
    self.userFilters = [NSMutableArray array];

    // This is Fast Enumeration
    for (SearchCriteria *sc in self.searchFilters)
    {
        if(sc.enabled)
        {
            [self.userFilters addObject:sc];
        }
    }
}

Answer 4

Just came back from lunch and already have a lot of answer, thank you very much. I need some time to read all the answers and test in my application.