I sometimes talk to myself. Our classes often do. But there are a couple of places where doing so is risky in Objective-C: init and dealloc.
This post is part of the Code Smells in Objective-C series.
Here’s something I see a fair bit in code for Objective-C init and dealloc. I’ll give a simple example. Can you tell what’s wrong?
- (id)initWithFoo:(id)foo
{
self = [super init];
if (self)
self.something = foo;
return self;
}
- (void)dealloc
{
self.something = nil;
[super dealloc];
}Hint: It’s those self-dots. They have a tendency to lull people into thinking they’re simple assignments. But remember, dot notation conceals messaging.
Let’s avoid dot notation and try again:
- (id)initWithFoo:(id)foo
{
self = [super init];
if (self)
[self setSomething:foo];
return self;
}
- (void)dealloc
{
[self setSomething:nil];
[super dealloc];
}Now do you see it?
When Messages to self Are Smelly
Sending messages to self is normally fine. But there are two places you want to avoid them:
- When the object is being created, and
- When the object is being destroyed.
At these two times, the object is in a funny, in-between state. It lacks integrity. Calling methods during these times is a code smell. Why? Because every method should maintain invariants as it operates on the object. Here’s the outline of an object’s self-consistency as it flows through a method:
- Start: Assume object is self-consistent.
- In-progress: Object state is in flux.
- End: Restore invariant that object is self-consistent.
#ProTip: Invariants will keep you sane.
I’m not striking out to unconventional territory for this. Apple has a document on Practical Memory Management that has a section titled, “Don’t Use Accessor Methods in Initializer Methods and dealloc.”
Objective-C init/dealloc: ivars to the Rescue
The solution is simple: in Objective-C init and dealloc methods, access instance variables directly instead of going through properties. In non-ARC code, check your property attributes for retain or assign. Then code your direct access to match. For example, if something is a retain property with the default backing ivar _something, our code becomes:
- (id)initWithFoo:(id)foo
{
self = [super init];
if (self)
_something = [foo retain];
return self;
}
- (void)dealloc
{
[_something release];
[super dealloc];
}When Messaging self in init/dealloc Can Still Work
Having said “avoid sending messages to self in init and dealloc,” I now want to soften that statement. There are two places when it might be OK after all:
- At the very end of init, and
- At the very beginning of dealloc.
That’s because in those two places, the object has self-consistency. In init, all the ivars have been set up. In dealloc, none of the ivars have yet been destroyed.
But you still have to exercise caution and recognize where you are in the object’s lifetime. Simply creating an object shouldn’t start any heavy-duty work. Keep creation and destruction light and quick.
What do you think? Share your thoughts and experiences in the comments below!
Did you find this useful? Subscribe today to get regular posts on clean iOS code.
Hi! Reasonable arguments, but why not just write _all_ other code in consistency ignoring manner? If you have too many relations and connected states inside single object – this is real smell. Avoiding this problem, messaging in undefined state in this case, will mask real problem.
Also properties are some kind incapsulated with their own setter/getter, and using ivars for direct access broke this incapsulation.
I prefer use properties 99% of time, and access to variables only in setter/getter methods. This allow code be more loose coupled and increase global semantic value of class. What are you think about this case?
Hi Alexey,
If understand you correctly, you’re saying use setters/getters everywhere, even during object construction/destruction, for consistency. I guess for me, I’m more interested in communicating the context than in 100% consistency. And the context of object construction/destruction is very different from anything else in an object’s lifetime.
For example, let’s say we have an object that calls setters/getters during construction/destruction. Technically, this isn’t really a problem for auto-generated setters/getters. But time goes on. Another programmer comes in. He or she makes a reasonable decision that a particular setter should do other clean-up work because of the state change. Suddenly, that other work is being invoked during construction/destruction!
Your counterargument is a good one: If this causes problems, it’s likely because the class has too much state, which masks the problem. I agree. But the hard reality of my work is that most of the code was written over many years by people who didn’t understand the importance of the Single Responsibility Principle.
As for using self-setters/getters in other methods, I go against the flow. Most people like to do as you describe, for safety. But because my code has unit tests to verify its functionality, I prefer to start with direct access. As Martin Fowler writes in his Refactoring book under “Self Encapsulate Field”:
What about setting inherited properties in an initialiser? Let’s say I’ve subclassed UIWindow. Can the “code smell” be removed from this init method?:
- (id)init{
self = [super initWithFrame:[[UIScreen mainScreen] bounds]];
if (self)
{
[self setWindowLevel:UIWindowLevelStatusBar];
[self setHidden:YES];
[self setUserInteractionEnabled:NO];
[self setBackgroundColor:[UIColor clearColor]];
}
return self;
}
Bryan,
Setting inherited properties is not a problem. But I’d do that towards the end of init once the object has self-consistency.
I really haven’t had a problem with that. Could you prove it with an example of where this might fail?
It won’t fail for simple properties where the underlying methods are auto-generated. It will fail as soon as you write your own method that depends on the state of the class, and invoke it while that state lacks self-consistency.
I don’t really agree.
I can understands why apple doesn’t want you to use accessors. It may not want you to trigger KVO during initialization.
After allocation, if I’m not wrong, all ivars are nil’ed (or after [NSObject init])
self is writable in init method family. After self = [super init] you checks if the self is set to nil and if its not you are safe do initialization.
At this point you are relatively safe to send message to self. Apple applies [self commonInit] for sharing common init between ns coding and other init.
My point is there’s a difference between messaging methods and using accessors. So I felt the title is quite misleading in my opinion.
Hi Stan,
I’m trying, and failing, to understand your point. Accessors — whether hand-rolled or synthesized — are simply methods. They’re dispatched in response to messages, just like anything else. Is dot-notation fooling you into thinking that messaging isn’t involved? (See my post, Dot Notation in Objective-C: 100% Pure Evil.)
Yes, zero is the starting point for all ivars. (So pointers to objects start as nil.) But there’s a difference between the self-consistent state of a single ivar, and the self-consistent state of the object as a whole.
Let’s say you made a class. In the course of initializing an object, it calls setters instead of directly manipulating ivars. As long as those setters are purely synthesized, there is no problem.
Now I come along with a modification: when a particular setter is invoked, I want it to post a notification so that observers can respond. So I write the setter. Suddenly, when an object is still in the middle of its init routine, it’s posting a notification and other objects are taking notice. And those objects, of course, assume that your object is self-consistent. But depending on the order of calls in the initializer, it may or may not be!
All this is avoided if you stick to ivars in init. Sure, you can extract a commonInit routine. Then make sure that it sticks to manipulating ivars.
Did that make sense?
> Suddenly, when an object is still in the middle of its init routine, it’s posting a notification and other objects are taking notice.
How could this possibly happen? In init your object identity has not yet been exposed to the outside, so no other object could even have subscribed using KVO or something similar.
My example is contrived, but I mean notification (as in NSNotificationCenter), not KVO.
I don’t see how this should cause real problems either. Using NSNotificationCenter one can post general notifications (without exposing a sender) or notifications tailored to one specific sender (exposing the sender).
General notifications are not dangerous, because they do not expose the identity of the object being init’ed. The ones with a sender are neither, because as in my KVO example no one could have subscribed to them specifically yet.
Also I don’t think I have ever seen a class that posts notifications during its initialization. Feels like a very artificial scenario to me.
So one (big) thing you lose from accessing ivars directly is validation (range checking, etc.) and cleaning / formatting of data (rounding, removing whitespace, etc.). Wondering how you address this limitation.
Bob, that’s an interesting thing to consider. Remember that we’re talking about the initialization method. I’d probably go one of two ways:
1) Extract the validation/cleaning, independent of the underlying ivar, as a class method (or even a C function).
2) Bite the bullet and say, “Eh, I’ll make an exception and call the setter.”
But the problem of valid data is exactly what I mean when I talk about self-consistency. Let’s say we’re tracking some kind of state in which zero is semantically invalid. (This could be a zero value or a nil pointer.) By trying to keep the init method from sending messages to self, we avoid the risk of going off to a method that will fail because the object isn’t done setting itself up.
On a ARC environment would be what? Retain wouldn’t be a solution.
Sorry, I don’t quite understand. Can you give a code example of your question?
In an ARC environment, you can’t use [myVar retain]…
retain’ is unavailable: not available in automatic reference counting mode
So what would you do in this case?
In ARC, the instance variables themselves have ARC characteristics. So in an initializer, do
_something = foo;
instead of
_something = [foo retain];
I assume that self-consistency is established by the time awakeFromNib is called?
Yes, self-consistency is established some time before the end of the initializer. So it’s certainly established before awakeFromNib.
How about the [self class], say for invoking class methods? I would think this safe.
Yes, I’d say that’s quite safe since nothing needs to be fully formed in the instance.
Would it be problematic to extract a lengthy portion of the init method into an instance method, so long as that instance method doesn’t access any iVars? For example:
- (id)init
{
self = [super init];
if (self) {
_oneButton = [self numberedButtonWithNumber:1];
_twoButton = [self numberedButtonWithNumber:2];
_threeButton = [self numberedButtonWithNumber:3];
}
return self;
}
- (UIButton *)numberedButtonWithNumber:(NSNumber *)number
{
UIButton *numberedButton = [[UIButton alloc] init];
[numberedButton setTitle:[number stringValue]
forState:UIControlStateNormal];
[numberedButton addTarget:self
action:@selector(pressNumberedButton:)
forControlEvents:UIControlEventTouchUpInside];
return numberedButton;
}
Bryan, a potential problem with that is that someone else could come along and add ivar use to that method. It would be safer to make it a class method instead.
Jon, i’m sorry, i don’t understand what exactly this thing self-consistency is. Can you please tell a little bit more about it?
Andrey,
By “self-consistency,” I mean the state of the object makes sense. Say you have an object with two NSNumbers for width & height. To be self-consistent, both width & height must be present. If either one is nil, the object doesn’t make sense. That’s the danger zone I’m trying to avoid.
I can see why it’s potentially bad news to use “self-dot…” to set properties, but what about calling other methods in “init”? Like this:
-(id)init
{
if ((self = [super init])) {
[self do something];
[self doAnotherThing];
}
return self;
}
Forgive my ignorance, but for generic objects (e.g. NOT view controllers that have lots of lifecycle methods like “viewDidLoad” that can be used to configure them) how else do you trigger other methods when an object is instantiated? It seems to me that the only jumping-off point you’re given is init? Have I been doing things wrong? Yikes!
Reid (nice name!),
Once the object’s state is self-consistent (like having all required properties set), then it’s totally safe to call other methods from init. You’re okay. :)
> Start: Assume object is self-consistent.
> In-progress: Object state is in flux.
> End: Restore invariant that object is self-consistent.
By this logic, calling methods inside other methods is a code smell. Let’s not take invariants to such a silly extreme. Apple documentation doesn’t actually say *why* we should avoid using setters in methods. I don’t know why exactly, but my guess is it’s because of how ObjC used to handle setters before ARC:
– (void)setCount:(NSNumber *)newCount {
[newCount retain];
[_count release];
// Make the new assignment.
_count = newCount;
}
https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/mmPractical.html
So if you’d write something like `self.foo = [[Foo alloc] init]`, without releasing it, you’d get a memory leak. I guess that’s the reason, but I really don’t know. The reason you give sounds sketchy to me.
None of this matters with auto-generated accessors. But now add extra behavior with a customized accessor. Then you’re skating on thin ice in init/dealloc. Rather than read through all the property implementations to decide which are or aren’t safe to call from init/dealloc, I favor avoiding them.
Sometimes you need an extra set-up or tear-down method. Their purpose should be explicitly identified by their names.
Hi Reid,
too old post but still valuable,
“I want to thank you first for being an inspiration for me :).”
I have a small question regarding calling accessors/getters
is calling an accessor from another accessor is a good idea ? e.g
– (void)setFirstName:(NSString *)firstName {
_firstname = firstName ;
self.lastName = firstName; // ?!!
}
thank you .
This feels like a code smell to me. I think it indicates that a lower-level class should be extracted.
Hi, Jon,
I wrote a short article about ‘if (self)’ checks in a constructor and got a few links to this article.
As far I see the only reason why you should not use properties directly in ctor/dtor – is a side effects. Which leads to another question: why does setter/getter have side effects that may cause an unexpected behaviour?
As for me accessors/mutators with side effects is a code smell, though I assume that there are some cases when it’s acceptable.
To sum up my thoughts just want to say: there is no silver bullet and there is no correct answer for the question “Can I use properties inside of init/dealloc?”
It always depends.
Hi,
interesting article, thanks. However, it seems to be pretty common to do stuff like this UIView init method:
-(instancetype)initWithFrame:(CGRect)frame {
if (self = [super initWithFrame:frame]) {
self.backgroundColor = [UIColor blueColor];
self.alpha = 0.9;
self.layer.cornerRadius = 4.0;
//etc
}
return self;
}
Are there problems to be expected with this? If so, how could I do it better?
Thanks and regards,
Sebastian Mecklenburg
That’s fine because those properties are in the superclass, which is fully formed because its `initWithFrame:` has completed.
How could it not complete?
It would not be complete before you call the superclass init.
This article needs to be fixed! You are contradicting yourself. You start with an example in which messages ARE “at the very end of init”, and “at the very beginning of dealloc” and you say that this is wrong, and at the end of the article you’re telling that some cases are fine, but those cases are exactly like the ones in the example from the beginning.
I might restate things like this: separate construction from work. “Avoid messages to self in init/dealloc” still stands.
This article (and the comments) have made this stuff click in my mind for the first time. Instead of just saying “because Paul Haggerty said so” now I understand why. Thank you.
Will, I had to search “Paul Haggerty Objective-C”. Heh! I’m glad to see this old post is still helpful.
Excuse me, but how could you make it when the property is declared as atomic? This case just happened to me.thx
Can you explain more about what happened?