[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Alloc/dealloc etiquette

From: David Phillip Oster
Subject: Re: Alloc/dealloc etiquette
Date: Sat, 17 Feb 2007 19:54:34 GMT
User-agent: MT-NewsWatcher/3.4 (PPC Mac OS X)

In article <>,
 Sherm Pendley <> wrote:

> A minor nit - I prefer to release instance variables (if they're objects)
> in their accessor methods. One common way to do this is with the accessors
> that can be automatically generated by Xcode:
>     - (void)setFoo:(NSObject *)value {
>         if (foo != value) {
>             [foo release];
>             foo = [value copy];
>         }
>     }

But that breaks encapsulation. Consider:

- (void)swapFoo:(FooOwner *)owner1 with:(FooOwner *)owner2 {
   NSObject *t = [owner1 foo];
   [owner1 setFoo:[owner2 foo]]; // <--
   [owner2 setFoo:t];

Once we get to the line with the comment above, "t" is dead, so on the 
next line, we attempt to assign a dead object to owner2.

It isn't reasonable to expect all client code to beware of this case, it 
isn't reasonable to expect all client code to be intimately familiar 
with the implementation of all setters. You should write:

- (void)setFoo:(NSObject *)value {
   [foo autorelease];
   foo = [value copy];

Shorter, simpler, more chance of being correct. Sure, it is slower, but 
I can make my programs as fast as I like if they don't have to be 

> To support this pattern, I don't use -release in -dealloc. Instead, I'd use
> the accessor there as well:
>     [self setFoo:nil];

No. Once again, client code would need to be intimately familiar with 
someone else's implementation. Setters often fire off notifications 
which trigger arbitrary code elsewhere in the program, or otherwise do 
other operations to get the object into a consistent state. None of that 
is appropriate inside -dealloc. Inside -dealloc, the object is dieing. 
It no longer is in a state where it should be firing off hoards of 
messages to preserve its consistency. Explicitly say what you mean:
- (void)dealloc {
   [foo release];
   foo = nil; // <- we nil it as a flag in case some other member of self
         // indirectly access it, in _their_ dealloc
   [super dealloc];

Don't depend on maintainers of -setFoo: checking every call everywhere 
in the program, to make sure they haven't broken some client code that 
was depending on setFoo: not doing anything else.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]