[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 <m2vei1wc52.fsf@local.wv-www.com>,
Sherm Pendley <spamtrap@dot-app.org> 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
correct.
> 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.
- Alloc/dealloc etiquette, Michael Hopkins, 2007/02/17
- Re: Alloc/dealloc etiquette, Michael Ash, 2007/02/17
- Re: Alloc/dealloc etiquette, Sherm Pendley, 2007/02/17
- Re: Alloc/dealloc etiquette,
David Phillip Oster <=
- Re: Alloc/dealloc etiquette, David Phillip Oster, 2007/02/17
- Re: Alloc/dealloc etiquette, Sherm Pendley, 2007/02/17
- Re: Alloc/dealloc etiquette, Michael Ash, 2007/02/18
- Re: Alloc/dealloc etiquette, David Phillip Oster, 2007/02/19
- Re: Alloc/dealloc etiquette, Michael Ash, 2007/02/19
Re: Alloc/dealloc etiquette, Michael Hopkins, 2007/02/17
Re: Alloc/dealloc etiquette, Jens Ayton, 2007/02/19