42

In @mmalc's response to this question he states that "In general you should not use accessor methods in dealloc (or init)." Why does mmalc say this?

The only really reasons I can think of are performance and avoiding unknown side-effects of @dynamic setters.

Discussion?

Community
  • 1
  • 1
schwa
  • 11,947
  • 14
  • 40
  • 54
  • It the OP hadn't ended with "Discussion?" this would not have been closed. It's a perfectly reasonable and useful question--eminently constructive. – Phil Sep 12 '14 at 22:56

6 Answers6

29

It's basically a guideline to minimize the potential for bugs.

In this case there is the (possibility) that your setter/getter may inadvertently make direct or indirect assumptions about the state of the object. These assumptions could be a problem when the object is in the midst of being setup or destroyed.

For example in the code below the observer does not know that 'Example' is being destroyed and could assume that other properties, which have already been freed, are valid.

(You could argue that your object should remove all observers before tearing itself down, which would be good practice, and another guideline to prevent inadvertent problems).

@implementation Example

-(void) setFoo:(Foo*)foo
{
   _foo = foo;
  [_observer onPropertyChange:self object:foo];
}

-(void) dealloc
{
   ...
   self.foo = nil;
}

@end
Andrew Grant
  • 56,758
  • 22
  • 127
  • 140
  • 2
    I see what you're saying but I'm not really buying it. The only real side effect is KVO getting fired while the object is in the midst of a dealloc. Is that really so bad? I've been doing this for a while (using the [self setFoo:NULL] style before objc2) and have yet to see a single problem. – schwa Oct 10 '08 at 21:37
  • 2
    I'd love for some sample code that can illustrate the problem - if there is one. Anyone up for it? :-) – schwa Oct 10 '08 at 21:39
  • Like I said, it's just a guideline to minimize the potential for problems. The same as how people recommend you set freed pointers to NULL. – Andrew Grant Oct 10 '08 at 22:58
  • though of course Apple reserve the underscore-namespace for its ivars, so your code sample has its own problems ;-) –  Oct 13 '08 at 03:12
  • How is using underscores to mark member variables a problem? I do it all the time, and have never seen a style doc or guideline saying otherwise... – rpj Oct 23 '08 at 15:10
  • 3
    see http://lists.apple.com/archives/cocoa-dev/2008/Jul/msg00808.html or http://lists.apple.com/archives/Cocoa-dev/2004/Jan/msg01684.html or http://developer.apple.com/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/NamingBasics.html#/ –  Dec 20 '08 at 15:55
  • This example is not valid. An object can not have an observer by the time it reaches its dealloc method. Otherwise you'll get an error. – Leibowitzn Oct 16 '09 at 19:01
  • I just found an example in some code where using the accessor caused a memory leak. The author had implemented a custom accessor method that rejected nil as a value. Thus, a member couldn't be nilled out at dealloc time, and a memory leak resulted. – Oscar Feb 17 '12 at 22:53
  • Andrew's answer is correct, in fact, Apple's documentation says not to use properties in init/dealloc: https://developer.apple.com/library/ios/documentation/cocoa/conceptual/MemoryMgmt/Articles/mmPractical.html#//apple_ref/doc/uid/TP40004447-SW6 – BergQuester Nov 12 '13 at 17:04
19

It is all about using idiomatically consistent code. If you pattern all of your code appropriately there are sets of rules that guarantee that using an accessor in init/dealloc is safe.

The big issue is that (as mmalc said) the code the sets up the properties default state should not go through an accessor because it leads to all sorts of nasty issues. The catch is that there is no reason init has to setup the default state of a property. For a number of reasons I have been moving to accessors that self initialize, like the simple example below:

- (NSMutableDictionary *) myMutableDict {
    if (!myMutableDict) {
        myMutableDict = [[NSMutableDictionary alloc] init];
    }

    return myMutableDict;
}

This style of property initialization allows one to defer a lot of init code that may not actually be necessary. In the above case init is not responsible for initing the properties state, and it is completely safe (even necessary) for one to use the accessors in the init method.

Admittedly this does impose additional restrictions on your code, for instance, subclasses with custom accessors for a property in the superclass must call the superclasses accessor, but those restrictions are not out of line with various other restrictions common in Cocoa.

Louis Gerbarg
  • 42,998
  • 8
  • 77
  • 88
  • 3
    Interesting point, but note that (unless you're assuming garbage collection?) the example you give leaves myMutableDict autoreleased... – mmalc Nov 19 '08 at 09:25
  • 7
    Additionally this way you can't assign `nil` to this property as the accessor will automatically create a new instance of an array. – Georg Schölly Nov 08 '09 at 21:24
  • That is one of the additional restrictions that would be imposed, and it doesn't invalidate the point. There are a number of situations where you never would assign nil to a particular property accept during teardown (in which case this would still work fine). I prime example of that would be an property that is redonly. – Louis Gerbarg Nov 08 '09 at 21:56
  • Also, while I am here, might as well edit it to make it safe for retain/release. – Louis Gerbarg Nov 08 '09 at 21:58
  • IMO, this code would be just dandy for a readonly property, not for a readwrite pair. Generally, I believe `object.x = foo` should imply that immediately afterwards `object.x == foo` is `YES`. If not, maybe (non-property) methods would serve better. – Clay Bridges Jan 05 '12 at 19:53
  • Localizing initialization to accessors is not a perfect practice. When establishing complex state relationships, order can be important. While you can certainly create separate initialization methods which can be triggered by accessors, you may need to add additional logic, (perhaps encapsulated within dispatch_once()) which is unnecessary if the initialization is called in init. I think that this practice is a bandaid which misses the crux of the problem -- state dependencies vary in nature and complexity and need to be reconciled by code maintainers and not by a simplistic coding regimen. – ctpenrose Sep 23 '13 at 16:54
15

You answered your own question:

  1. Performance may be a perfectly adequate reason in itself (especially if your accessors are atomic).
  2. You should avoid any side-effects that accessors may have.

The latter is particularly an issue if your class may be subclassed.

It's not clear, though, why this is addressed specifically at Objective-C 2 accessors? The same principles apply whether you use declared properties or write accessors yourself.

mmalc
  • 8,201
  • 3
  • 37
  • 39
2

It may be that the setter has logic that should run or perhaps the implementation used an ivar with name different from the getter/setter or perhaps two ivars that need to be released and/or have their value set to nil. The only sure way is to call the setter. It is the setter's responsibility to be written in such a way that undesirable side effects do not occur when called during init or dealloc.

From "Cocoa Design Patterns", Buck, Yacktman, pp 115: "... there is no practical alternative to using accessors when you use synthesized instance variables with the modern Objective-C runtime or ..."

zaph
  • 108,117
  • 19
  • 176
  • 215
  • In my question on this (http://stackoverflow.com/questions/1283419), one of the answers revealed that you can still access ivar directly even if you're just declaring the property (a synthesized ivar). – Dave DeLong Oct 16 '09 at 18:30
  • @Dave Accessing the ivar directly assumes that you know it's name and that there is only one ivar that the setter changes. But one is not supposed to know (or care) how a setter is implemented. – zaph Oct 16 '09 at 19:44
  • Setters/Getters can be overriden. They can release other resources (e.g. observers). – Sulthan Dec 09 '11 at 20:49
0

In fact, for a class that comes and goes rather often (like a detail view controller), you want to use the accessor in the init; otherwise, you could end up releasing a value in viewDidUnload that you try to access later (they show that in CS193P...)

FeifanZ
  • 15,982
  • 6
  • 44
  • 80
0

You can create the same problems by NOT calling the setter when allocating/deallocating.

I don't think you can achieve anything by using retain/release directly in init/dealloc. You just change the set of possible bugs.

Everytime you have to think about the order of property allocation/deallocation.

Sulthan
  • 118,286
  • 20
  • 194
  • 245