Difference between creating a local variable and assigning to ivar and directly assigning to ivar?

Question

Hello,

I have always wondered why all apple code samples use code like this:

UINavigationController *aNavigationController = [[UINavigationController alloc]
          initWithRootViewController:rootViewController];

self.navigationController = aNavigationController;

[self.view addSubview:[navigationController view]];

[aNavigationController release];

They always create a local variable and assign it to the ivar why don't they simply do this:

self.navigationController = [[UINavigationController alloc]
          initWithRootViewController:rootViewController];;

[self.view addSubview:[navigationController view]];

[navigationController release];

Is there any other reason, besides it is easier to understand?. Is this a best practice?.

-Oscar

Answer 1

The differences in the first lines is that Apple's version separates object creation and assignment to an ivar whereas yours puts the the two together. Conceptually, Apple's version is slightly easier to understand. It is not a best practice to my knowledge.

Answer 2

Both versions miss a check for a nil value:

(Assuming self.navigationController is a property that retains it's value)

self.navigationController = [[UINavigationController alloc]
    initWithRootViewController:rootViewController];
if (self.navigationController != nil) {
    [self.view addSubview: navigationController.view;
    [self.navigationController release];
}

You could argue this is style, but in my opinion it leads to less buggy code.

Answer 3

It may be the same thing as the top example, but there's a chance it won't be.

Remember that

self.navigationController = aNavigationController;

is the same as

[self setNavigationController:aNavigationController];

and you don't know what's going on inside that setNavigationController method. It could be initializing a different object and setting that as the iVar, which you then release, causing a crash.

Answer 4

Since it's clear that the code is using a UINavigationController instance variable. Then there wouldn't be no reason to do:

self.navigationController = aNavigationController

If you're not retaining it.

But, if you do it like this:

 self.navigationController = [[UINavigationController alloc] initWithRootViewController:rootViewController];

Afterwards, if you release it like this:

[navigationController release];

It will appear to be that we are releasing the instance variable that's supposed to be retained for the lifetime of the current class that initialize the navigation controller. So, it's prone to mistake, that will make beginners think that it should only be released in the dealloc method.

Both approach will have retain count of 0 in the end. If at the dealloc implementation:

[navigationController release]; // 1 for the ivar
[super dealloc]; // 0 for the retained subviews

Answer 5

Your replacement code is incorrect, and thus illustrates the problem that Apple is trying to prevent. Here is your code:

self.navigationController = [[UINavigationController alloc]
      initWithRootViewController:rootViewController];

[self.view addSubview:[navigationController view]];

[navigationController release];

You left out the "self." in your references. Perhaps you meant to access the ivar directly, but in that case you have created very confusing code by mixing accessors and direct ivar access (and violated the cardinal rule by using direct ivar access outside an accessor). If not, then you meant to write this:

self.navigationController = [[UINavigationController alloc]
      initWithRootViewController:rootViewController];

[self.view addSubview:[self.navigationController view]];

[self.navigationController release];

That last line is very wrong. Never send -release to the result of a method call. So no, the way you're doing it isn't correct.

That said, Apple and I disagree on how to do this. Here's how I do it:

self.navigationController = [[[UINavigationController alloc]
      initWithRootViewController:rootViewController] autorelease;

[self.view addSubview:[self.navigationController view]];

I like -autorelease because I find it prevents errors. The further apart the alloc and release get, the more likely a developer will inject a memory leak (by adding a "return" for instance). autorelease avoids this by keeping the retain and release together, making the intent to use this as a temporary variable more clear, and generally makes code review much easier.

Apple tends to disagree with me in their example code because they're emphasizing performance by using release versus autorelease. I find this to be the wrong optimization, since this object won't be deallocated during this run loop in either case (so no memory is saved), and I believe the very small performance penalty of autorelease is more than made up for by the reduction in memory leaks due to incorrect use of release.

The autorelease vs. release debate is filled with shades of gray (I certainly use release directly in loops), and different developers have different approaches, but your replacement code isn't the right way to do it in either case.