views:

27

answers:

2

Hi there,

Sorry for the long title. In essence, I want to accomplish the same thing that the Calendar app does for Event details. The first cell shows the title and date of the event. The second shows an alert, if there is one, otherwise Notes, if there are any, or nothing else if none of these fields is present.

The way I am doing it now is a really long if condition in cellForRowAtIndexPath:

if(indexPath.row == 0) {
  TitleCell *titlecell = [[TitleCell alloc] init];
  // config cell to do title here, always
  return titlecell;
} else if (indexPath.row == 1 && foo) {
  FooCell *foocell = [[FooCell alloc] init];
  // config cell to show foo, if it exists
  return foocell;
} else if (indexPath.row == 1 && bar) {
  BarCell *barcell = [[BarCell alloc] init];
  // foo doesn't exist, but bar, so show bar in cell 1
  return barcell;
} // etc etc

That's really ugly, and since I create the cells in the if and return, the static analyzer tell me that each one of those is a potential leak. There is no else, since I need to cover all scenarios anyway, and that also gives a warning about the method potentially not returning anything.

Is there a better way that makes this cleaner and doesn't give me warnings?

Thanks!

Christoph

+1  A: 

Clang is right when it says that you're leaking memory - The UITableView retains the UITableViewCells that you give it, so you should be autoreleasing them in cellForRowAtIndexPath.

Instead of using if statments, I think you should be using a switch.

Jacob Relkin
Oh wow, you're right. Must've missed the autorelease in the Xcode template and just didn't put it back when I changed stuff around. Thanks!
Christoph
+2  A: 

The warning are because you are leaking memory, you have to autorelease the cell: TitleCell *titlecell = [[[TitleCell alloc] init] autorelease];. Also there is a chance of not having a return statement because you don't have else in your if block.

Here is another way of doing it:


// Call this beforehand
- (void)loadTable {
  // Since we are not using objects, we need to use a non-retaining array
  // A better way of doing this would be with delegates or NSInvocations
  array = (NSMutableArray*)CFArrayCreateMutable(NULL, 0, NULL); 
  [array addObject:(id)@selector(buildTitleCell)];
  if (foo)
    [array addObject:(id)@selector(buildFooCell)];
  if (bar)
    [array addObject:(id)@selector(buildBarCell)];
}

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath {
  if (indexPath.row < array.count) {
    SEL selector = (SEL)[array objectAtIndex:indexPath.row];
    return [self performSelector:selector];
  }
  return nil;
}

- (UITableViewCell*)buildTitleCell {
  TitleCell *titlecell = [[[TitleCell alloc] init] autorelease];
  // config cell to do title here, always
  return titlecell;
}
...

EDIT: fixed as per @Christoph's comment

Aleph
Thanks, though I am not sure that'll work if Foo doesn't exist. In that case, Bar should be in cell 1. Or am I missing something?
Christoph
You are right, I fixed it now.
Aleph
Thanks both of you for pointing out I am, in fact, leaking memory. I picked this as the accepted answer for its code.
Christoph