UITableView custom class huge leak

Question

Hi,

I am using a custom class to display some info on a table view. The problem is that as long as I scroll the tableview memory is leaking...

I guess I have something wrong at my class.

Please have a look:

    @interface Person : NSObject {

 NSString *name;
 NSString *surname;
 NSString *address;
 NSString *email;

}

@property (nonatomic, copy) NSString *name, *surname, *address, *email;



@implementation Person
@synthesize name, surname, address, email;

-(id)init {

 [super init];
 name = [[NSString alloc] init];
 surname = [[NSString alloc] init];
 address = [[NSString alloc] init];
 email = [[NSString alloc] init];
 return self;
}



- (void)dealloc
{
 [name release];
 [surname release];
 [address release];
 [email release];
 [super dealloc];
}


#import "Person.h"

@interface Group : NSObject {

 NSString *groupTitle;
 NSMutableArray *persons;

}

@property (readwrite, copy) NSString *groupTitle;

- (void)addPerson:(Person *)person;
- (void)removeAll;
- (NSArray *)getPersons;
- (int)PersonsCount;

@end




@implementation Group
@synthesize groupTitle;



-(id)init {

 [super init];
 persons = [[NSMutableArray alloc] init];
 return self;
}


-(void)addPerson:(Person *)person {


 [persons addObject:person];

}

-(void)removeAll {

 [persons removeAllObjects];

}

-(NSArray *) getPersons {

 return [persons copy];
 [persons release];


}

-(int)personsCount {

 return [persons count];

}

-(void)dealloc {

 [groupTitle release], groupTitle = nil;
 [persons release], persons = nil;
 [super dealloc];
}


@end




- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath {

  …….


  Group *groupForRow = [[Group alloc] init];
  Person *personForRow = [[Person alloc] init];
  personForRow = [[groupForRow getPersons] objectAtIndex:indexPath.row];

  _personName = personForRow.name;
  _personSurname = personForRow.surname;
  _personAddress = personForRow.address;
  _personEmail = personForRow.email;


  [groupForRow release], groupForRow = nil;
  [personForRow release], personForRow = nil;

  …..

  return cell

Answer 1

There is a lot wrong here, please delve a little into objective-C to get a grasp of the use of @property and @synthesize to get correctly functioning getter/setter methods.

As for your memory leak when scrolling, it is caused by the allocs in cellForRowAtIndexPath which are not balanced by either a release or an autorelease.

This:

Group *groupForRow = [[[Group alloc] init] autorelease];
Person *personForRow = [[[Person alloc] init] autorelease];

should fix most of your leaks. Browse around on SO for more info.

Answer 2

Few corrections (read the comments):

@interface Person : NSObject {
    NSString *name;
    NSString *surname;
    NSString *address;
    NSString *email;
}

// copy is OK for strings...
@property (nonatomic, copy) NSString *name, *surname, *address, *email;

@end


@implementation Person

@synthesize name, surname, address, email;

- (id)init {
    if (self = [super init]) {
        // There is no need to allocate the strings
        // In addition, once you write 'name = [[NSStrin alloc] init];' you don't use the property.
        // If you do want to use the property setter then you should write 'self.name = @"some string";'
    }
    return self;
}


- (void)dealloc {
    [name release];
    [surname release];
    [address release];
    [email release];
    [super dealloc];
}

@end


#import "Person.h"

@interface Group : NSObject {
    NSString *groupTitle;
    NSMutableArray *persons;
}

// Any special reason for "readwrite" instead of "nonatomic"?
@property (readwrite, copy) NSString *groupTitle;
// This property is more important than the string:
@property (nonatomic, retain) NSMutableArray *persons;

- (void)addPerson:(Person *)person;
- (void)removeAll;
- (NSArray *)getPersons;
- (int)PersonsCount;

@end


@implementation Group

@synthesize groupTitle, persons;

- (id)init {
    if (self = [super init]) {
        // Use the autoreleased array instance ([NSMutableArray array]) and set it to the property setter that will retain the object:
        self.persons = [NSMutableArray array];
    }
    return self;
}


- (void)addPerson:(Person *)person {
    // I prefer using properties (the "self." in the beginning) instead of the members directly...
    [self.persons addObject:person];
}

- (void)removeAll {
    [self.persons removeAllObjects];
}

// I think that this getter is unnecessary - use the property instead...
- (NSArray *) getPersons {
    // There is no need to copy
    return [persons copy];
    // Don't you have a warning for this line? It is never executed
    [persons release];
}

- (int)personsCount {
    return [self.persons count];
}

- (void)dealloc {
    [groupTitle release], groupTitle = nil;// The "groupTitle = nil" is unnecessary.
    [persons release], persons = nil;// The "persons = nil" is unnecessary.
    [super dealloc];
}

@end


- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath {

    …….


    Group *groupForRow = [[Group alloc] init];// Do you REALLY have to allocate this object each "cellForRowAtIndexPath"??
    Person *personForRow = [[Person alloc] init];// Get rid of the "= [[Person alloc] init]" - this is a leak (because of the next line)
    personForRow = [[groupForRow getPersons] objectAtIndex:indexPath.row];// If you will use the property persons instead of the "getPersons" (that copies the array) then you will get rid of another leak

    // What are these?
    _personName = personForRow.name;
    _personSurname = personForRow.surname;
    _personAddress = personForRow.address;
    _personEmail = personForRow.email;

    // The " = nil" is unnecessary here...
    [groupForRow release], groupForRow = nil;// If you won't allocate the group then you won't need this line...
    [personForRow release], personForRow = nil;// NSZombie - you release object that you don't owe (do you have crashes, that you don't know why they are happen?)

    …..

    return cell;
}