Tutorial :Objective-C array of objects misbehaving (EXC_BAD_ACCESS) in TableViewController



Question:

I'm new to Objective-C, and I can't figure out why the NSString objects in my array of Car objects seem to have been released.

Here's my Car.m class:

  #import "Car.h"  @implementation Car  @synthesize categoryId;    - (id)initWithPrimaryKey:(NSInteger)pk categoryId:(NSNumber *)catId carName:(NSString *)n {      if (self = [super init]) {          primaryKey = pk;          categoryId = catId;          name = n;      }        return self;  }  - (void)dealloc {      [name release];      [categoryId release];        [super dealloc];  }  - (NSInteger)primaryKey {      return primaryKey;  }  - (NSString *)name {      return name;  }  - (void)setName:(NSString *)aString {      if ((!name && !aString) || (name && aString && [name isEqualToString:aString])) return;      [name release];      name = [aString copy];  }    @end  

And here's my Simple_TableViewController.m class. The listData instance variable is set correctly in viewDidLoad(). In the debugger, the NSString *name property of each array element is intact. Then, in every other method, listData is corrupt. It has all the Car elements with the correct primaryKey and categoryId values, but the NSString *name property is "Invalid."

  #import "Simple_TableViewController.h"  @implementation Simple_TableViewController    - (void)viewDidLoad {             NSMutableArray *array = [[NSMutableArray alloc] init];      Database *database = [Database instance];        array = [database getAllCars];      [self setListData:array];      [array release];  }  - (NSMutableArray *)listData {      return listData;  }  - (void)setListData:(NSMutableArray *)newListData {      if (listData != newListData) {          [listData release];          listData = [newListData mutableCopy];      }  }  - (BOOL)shouldAutorotateToInterfaceOrientation:(UIInterfaceOrientation)interfaceOrientation {      return (interfaceOrientation == UIInterfaceOrientationPortrait);  }  - (void)didReceiveMemoryWarning {      [super didReceiveMemoryWarning];  }  - (void)dealloc {      [listData release];      [super dealloc];  }  - (NSInteger)tableView:(UITableView *)tableView   numberOfRowsInSection:(NSInteger)section  {      return [listData count];  }  - (UITableViewCell *)tableView:(UITableView *)tableView              cellForRowAtIndexPath:(NSIndexPath *)indexPath  {      static NSString *SimpleTableIdentifier = @"SimpleTableIdentifier";        UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:SimpleTableIdentifier];      if (cell == nil) {          cell = [[[UITableViewCell alloc] initWithFrame:CGRectZero                                                   reuseIdentifier:SimpleTableIdentifier] autorelease];      }        NSUInteger row = [indexPath row];      Car *car = [listData objectAtIndex:row];        cell.text = car.name;      cell.font = [UIFont boldSystemFontOfSize:17];        return cell;  }  - (NSInteger)tableView:(UITableView *)tableView  indentationLevelForRowAtIndexPath:(NSIndexPath *)indexPath  {      return 0;  }  - (NSIndexPath *)tableView:(UITableView *)tableView    willSelectRowAtIndexPath:(NSIndexPath *)indexPath  {      NSInteger row = [indexPath row];      if (row == 0) {          return nil;      }        return indexPath;  }  - (void)tableView:(UITableView *)tableView  didSelectRowAtIndexPath:(NSIndexPath *)indexPath  {      NSUInteger row = [indexPath row];        NSString *message = [[NSString alloc] initWithFormat:@"You selected %@", [[listData objectAtIndex:row] name]];      UIAlertView *alert = [[UIAlertView alloc] initWithTitle:@"Row Selected!"                                                                      message:message                                                                    delegate:nil                                                        cancelButtonTitle:@"Yes I Did"                                                        otherButtonTitles:nil];      [alert show];        [message release];      [alert release];  }  - (CGFloat)tableView:(UITableView *)tableView heightForRowAtIndexPath:(NSIndexPath *)indexPath  {      return 35;  }    @end  

I've fiddled with Zombie this and retain that until my fingers fell off. I did away with the @property (nonatomic, retain) and @synthesize stuff for the listData variable, and I still have the same problem.

Thanks in advance for any advice you have to offer!


Solution:1

In your initWithPrimaryKey:etc: method, you need to retain those values, or use the dot notation to assign them (assuming they are properties declared with the retain keyword). Try this:

  - (id)initWithPrimaryKey:(NSInteger)pk categoryId:(NSNumber *)catId carName:(NSString *)n {      if (self = [super init]) {          primaryKey = pk;          categoryId =[catId retain];          name = [n retain];      }

return self;  

}

Also, and this is just a style issue, you might consider slightly more verbose names than 'n' and 'pk'. Your future-self will thank you.


Solution:2

Slightly off topic - Ben has the answer you are looking for but the logic in some of your code is a little tortuous.

- (void)setName:(NSString *)aString {      if ((!name && !aString) || (name && aString && [name isEqualToString:aString])) return;      [name release];      name = [aString copy];  }  

You don't really need to check (!name && !aString) it's safe and common practice to reassign nil over nil and to send a release message (or any message) to nil. The runtime short circuits the latter and the optimiser will remove the former. If setting a nil name is an error, you should assert that aString is not nil

Similarly, NSStrings are very well optimied. Don't bother optimising out the case where you are setting the name of a car to the name that is already set. Infact, your check [name isEqualToString:aString] which you take every time someone set a non-nil string is slower than the release and copy you are avoiding in the minority of cases.

If you check what happens when you send copy to an NSString, you'll find it just increases the retain count. Note that this is not true for NSMutableString for obvious reasons ;-)

A good rule of thumb - optimise when you have all functionality and only optimise what is slow (or big).

finally, following on from Ben's comments about naming, I'd rename aString to aCarName or something else more descriptive - again, your future self will thank you.


Note:If u also have question or solution just comment us below or mail us on toontricks1994@gmail.com
Previous
Next Post »