Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Bad code on Apple sample code?

I've looked at the Apple "PageControl" sample project (See here) and I see this function in the code :

- (void)loadScrollViewWithPage:(int)page
{
    if (page < 0)
        return;
    if (page >= kNumberOfPages)
        return;

    // replace the placeholder if necessary
    MyViewController *controller = [viewControllers objectAtIndex:page];
    if ((NSNull *)controller == [NSNull null])
    {
        controller = [[MyViewController alloc] initWithPageNumber:page];
        [viewControllers replaceObjectAtIndex:page withObject:controller];
        [controller release];
    }

    // add the controller's view to the scroll view
    if (controller.view.superview == nil)
    {
        CGRect frame = scrollView.frame;
        frame.origin.x = frame.size.width * page;
        frame.origin.y = 0;
        controller.view.frame = frame;
        [scrollView addSubview:controller.view];

        NSDictionary *numberItem = [self.contentList objectAtIndex:page];
        controller.numberImage.image = [UIImage imageNamed:[numberItem valueForKey:ImageKey]];
        controller.numberTitle.text = [numberItem valueForKey:NameKey];
    }
}

There is something I don't understand.
If the test part if ((NSNull *)controller == [NSNull null]) is true, then we have

controller = [[MyViewController alloc] initWithPageNumber:page];
[viewControllers replaceObjectAtIndex:page withObject:controller];
[controller release];

Then just after :

if (controller.view.superview == nil)

But, controller has just been released. I think it may work because controller is still in memory, but isn't this a wrong way of coding the stuff?

like image 506
Oliver Avatar asked Mar 18 '11 23:03

Oliver


2 Answers

I think you're right. If I were coding this I would have added:

if ((NSNull *)controller == [NSNull null])
{
    controller = [[MyViewController alloc] initWithPageNumber:page];
    [viewControllers replaceObjectAtIndex:page withObject:controller];
    [controller release];
    controller = [viewControllers objectAtIndex:page];
}

To grab a reference after releasing.

Or I suppose the other way would be to use autorelease instead of release.

like image 116
NWCoder Avatar answered Sep 28 '22 15:09

NWCoder


When the added the controller object into viewControllers that retained the object. Therefore, it is still a live object after they release it.

Is it wrong? Maybe, it requires knowledge of what NSArray does, but that's fairly common.

I would have done it like so:

if ((NSNull *)controller == [NSNull null])
{
    controller = [[[MyViewController alloc] initWithPageNumber:page] autorelease];
    [viewControllers replaceObjectAtIndex:page withObject:controller];
    controller = [viewControllers objectAtIndex:page];
}
like image 39
MarkPowell Avatar answered Sep 28 '22 15:09

MarkPowell