Defensive Coding in Objective-C

When programming in a C-descended language like Objective C, there are many things that can easily go wrong. To avoid the worst of these errors, programmers have come up with various coding conventions that make it harder to cause such bugs. We're not talking about indentation or spacing, but rather about "mini-patterns" that ensure certain errors are caught more easily. Here's my spontaneous, certainly not exhaustive list:

Autorelease Early, Autorelease Often

When you allocate a new object that doesn't immediately go into an instance variable, it is easy to forget to release that object and leak it. Even if you remember to call -release on it at the end of your method, someone might later add a return statement somewhere, and overlook there's an object in need of releasing.

One way to fix this is to use goto and a bail: label to cause all exits from your method to go through one funnel point that releases everything again. Kind of a "dealloc method for your method". goto is not inherently bad (that's just a rumour brought about by a misinterpretation of the title of a paper by Mr. Dijkstra). That said, the code quickly becomes hairy if you have many different error exits from your method.

An easier way to fix this is to just remember to -autorelease the object right after you create it. That way, at the moment of creation, where it is glaringly obvious there's an object in need of later cleanup (based on alloc/init or copy in the name), you already ensure you're not leaking. If someone needs it later, they can always retain it explicitly. Leak-free code for free, and even for people with the attention span of a goldfish (Or poisson rouge, as my favorite leak-hunting colleague would say).

NIL Everything That Isn't Bolted Down

The problem with C is that local variables are not initialized to zero. Nor are pointers to released objects cleared to nil. No, local variables contain arbitrary numbers that happened to be on the stack when they were created, and variables valiantly keep pointing at the spot that used to house your NSString long after you've released it. "There! Look! There's a big bunch of nothing here that seems to be an NSString!"

A good way to avoid spending hours trying to track down dangling pointers is to set them to nil whenever they contain nothing. Every time you declare a pointer like

	NSString* myString;

stop yourself and instead initialize it properly

	NSString* myString = nil;

You'll be grateful you did that the moment someone adds an if statement around a few lines that used to assign a value to this variable.

The same applies to objects that you dispose of. The moment you dispose of an object, set the variable that used to point to it to nil (be it an instance variable, a global, or just a local one). Again, in a complex function, someone might insert an if statement that releases your object, and miss that under certain conditions, the code you wrote still tries to access that object. When you later debug that code, nil will make it obvious the object is gone. On the other hand, any old pointer, probably still pointing at valid-looking remnants of the object that used to be there, will not obviously be invalid to you.

I've defined myself a DESTROY() macro like GNUstep has it to help with this. DESTROY() first releases an object, then sets its variable to nil. But I only write DESTROY(myVar);.

Don't Use Accessors in Constructors or Destructors

This may sound a bit odd, but there is a reason to this madness. Constructors (i.e. -init methods in ObjC) and destructors (i.e. -dealloc or -finalize) are special methods: They are called before your object has fully been initialized, or may be called after it has already been partially torn down.

If someone subclasses your class, your object is still an object of that subclass. So, by the time your -dealloc method is called, the subclass has already been asked to do its -dealloc, and most of the instance variables are gone. If you now call an accessor, and that accessor does anything more than change the instance variable (e.g. send out notifications to interested parties), it might pass a pointer to its half-destructed zombie self to those other objects, or make decisions based on half-valid object state. The same applies to the constructor, but of course in reverse.

Now, some people that accessors should not be doing anything but change instance variables, but that is utter nonsense. If that was all they're supposed to do, we wouldn't need them. Accessors are supposed to maintain encapsulation. They're supposed to insulate you from the internal details of how a particular object does its work, so you can easily revise the object to work in a completely different way, without anyone on the outside noticing. If an accessor could only change an instance variable, you would have very limited means to change this internal implementation.

Moreover, I don't think Apple would have introduced Key-Value-Coding and Key-Value-Observing if they didn't agree at least partially that it's fine to do a bunch of things in response to an accessor being used to mutate an object.

Mind you, all of this only applies to accessors being called on self from your constructor. If you're setting up another object, you essentially have no choice but to use its accessors, and it would very often violate encapsulation if you did otherwise.

In Fact, Don't Do Anything Big in Constructors and Destructors

The above rule can actually be made more generic: Whenever you do anything in a constructor or a destructor, try to think whether you really need to do it here and now. They're mainly there to manage your instance variables. If you have to register for notifications or otherwise access external objects, it's always safer to do it elsewhere, when you can be sure that your object has been completely constructed.

A neat trick in constructors is to call -performSelector:withObject:afterDelay:0 on yourself. This will ensure a method to initialize stuff gets called on your object the next time through the event loop. Of course, for many objects that opens yet another can of worms (imagine you'd just created an NSScanner and had to wait for the event loop to run once before you can use it!).

Another thing that sometimes works is to access external objects lazily. E.g. the first time someone calls any of the -scanXXX methods on an NSScanner, it could transparently and implicitly do some more involved setup and set a flag that this setup has happened.

I have a similar recommendation for destructors: You should try to close files or relinquish external resources explicitly, before your object is released, if you can. There's nothing wrong in having code in your constructor that makes sure of this as well (i.e. to avoid leaking open file descriptors), but it is desirable to have that as a fallback, not as the preferred API.

Now, before you go all "goto considered harmful" on me: I'm not saying doing worthwhile things in constructors in destructors was bad. Rather, all I'm saying is that other options for good places to do it are often overlooked. Both because the whole matter of half-constructed/destructed objects can get hairy, and also because anyone else can retain your object and thus prevent your resource from going away.

If the object is by itself the resource, that is exactly what you want. It is what retain/release was designed for, after all. But if the resource simply represents a file or a hardware device, and someone deletes the file or unplugs the device, you must be able to cope gracefully with your object still existing because some nit retains it, even though the actual resource is gone.

And if you want to call methods that a subclass would want to override (and in Objective-C, there is no such thing as a "method that can not be overridden", by design!), you'd prefer to have a fully-initialized object ready.

Follow Apple's Singleton Design Pattern

There is a nice little example implementation of the Singleton design pattern on Apple's developer web site. Implement it.

While I think the -retain/-release methods should actually be left alone so you get some decent crashes and notice when someone retains/releases a singleton the wrong number of times (retaining or releasing should be allowed on any object, even if just to make it easy to keep certain code agnostic of the precise type it's dealing with, so we can't just make it throw an exception), they got a lot of details right:

They don't wait for -init to return to set the global singleton variable. After all, singletons can be subclassed, too (such a subclass usually gets instantiated instead of the superclass, as just like on Highlander, there can only be one). If any of the -init methods do anything that might trigger code that might in turn call your +sharedManager method (like, I don't know, register for IOKit notifications and send NSNotifications when they come in), this would invite endless recursion. Since the singleton global hasn't been set yet, that second call would create a second singleton instance, which would in turn trigger the notifications, which would in turn create a third singleton ... and so on.

What Apple's code does is to cleverly override +alloc to set the global variable. That way, it is already set before anyone ever gets around to doing anything with the object. They also have a lock on the class. So, even in a multi-threaded implementation, they only allocate the object once. Since they return nil on subsequent attempts to alloc the object, they also only ever allocate and init one object.

It's a very solid implementation, and whenever I've taken a shortcut on this in the past (and the code on my site will show you I have been doing this this until fairly recently), it's caused me pain. I'm glad I finally understand it now.

Clear Your Weak References

One convention in Cocoa is that you don't retain your delegate. This kind of "weak" reference from the delegator to the delegate may seem dangerous at first, but makes complete sense in the common use case:

Usually, a controller object creates another object and retains it, and sets itself up as that object's delegate to be able to modify or benefit from its behaviour. For example, an NSWindowController creates an NSWindow, becomes its owner by retaining it, and makes itself the delegate of that window.

Now, if the NSWindow retained its delegate, if it retained the NSWindowController, we would have a retain circle: When the NSWindowController is released by the last external party, it would still have a retain count of 1, because the NSWindow would have its delegate retained. However, the NSWindow would also have a retain count of 1, because the NSWindowController created the NSWindow and kept it retained. So both are waiting for the other to release them. Only then would their -dealloc methods get called, which would release the other one. They'd be like two lovers lost in space, separate from everyone else, but closely holding on to each other.

So, the rule was laid down: You don't retain your delegate, as the delegate probably already owns you. But what happens if someone else retains the object you own? Your NSWindowController is released, it relinquishes its hold on the NSWindow by releasing it. But that other guy still has the NSWindow retained, so it stays open. Someone clicks your window and a delegate method is called.

Wait a second! The NSWindowController was the delegate! But it is gone!? Well, unless the NSWindowController was considerate enough to tell the NSWindow, by calling its -setDelegate: method and setting it to nil, NSWindow wouldn't know. It would find itself yelling at a dead object, probably crashing.

So what have we learned? Unless you're a fan of zombies, you'll appreciate setting any weak references to yourself to nil in your -dealloc method.

In case you're wondering who might be mad enough to retain your objects, look no farther than the deferred method call mechanisms, particularly NSTimer, NSThread, NSInvocation and the -performSelector:... family of methods that eventually end up with your NSRunLoop.

Of course, you can go and invalidate the timer, cancel the -performSelector:s, and in many cases you well should, but in other cases, you may actually want all of these operations to be performed on the object before it goes away (though maybe not in our example of an NSWindow). And of course this isn't really a good example, because a good design would probably not install timers and the likes on objects but themselves (that usually violates encapsulation, after all). Then again, with NSInvocation you have no choice.

Use symbolic constants

Cocoa and Foundation makes use of string constants for identification purposes a lot. Notification names, keys in NSDictionary objects. You also use them elsewhere, to refer to files on disk, processes using their bundle identifier etc.

Now, everyone knows that defining a string constant as a symbolic constant using #define of by defining it as a variable at global scope makes it easier to change this string later. Particularly if the string is used in several places. But often, people "know" that this constant will never need to be changed, so they just hard-code it. Bad idea. There are other advantages to a symbolic constant:

The Compiler knows about symbolic constants.

That is right. That means that, should you mistype the symbolic constant, the compiler will only see an unknown identifier. If you mistype a regular string constant, all the compiler sees is a string. A compiler has no idea that "MyPrettyColor" and "MyPrettyColour" are supposed to be the same thing, but one of them is obviously a mis-spelling. If you had defined a symbolic constant like

#define MyPrettyColour    "MyPrettyColour"

It would compile to the exact same code as using the pure string constant, but if you mistype MyPrettyColour as MyPrettyColor, the compiler would immediately tell you about that and you wouldn't wonder why a dictionary value always gets returned as nil even though you know for certain you put it in the dictionary.

This applies similarly to any other kind of constant, be it an int, a double or whatever. It's easy to hit 111111 when you meant to write 11111, and that excess digit is not always easily noticed, as our mind tends to "correct" what we see as it tries to make sense of it. If you define a symbolic constant, the compiler will catch any additional letter you type by accident. Even better, you can define the constant correctly. Tend to forget the U at the end of unsigned numbers? The constant will always contain it, you only have to think of it once. If you forgot it, you can simply add it, and all other spots that use the constant are magically fixed.

And last but not least, symbolic constants can improve readability. Imagine drawing code where you deal with margins, line widths etc. Now in one spot you draw a button, and in another you hit-test it. To do hit-testing, you inset your rectangle. What do you think is more maintainable?

buttonHotRect = NSInsetRect( [self bounds], 67, 42 );

buttonHotRect = NSInsetRect( [self bounds],
                             MyLeftMarginWidth + MyLineWidth + MyLineWidth
                             + MyRightShadowWidth + MyRightMarginWidth,
                             MyTopMarginHeight + MyLineHeight + MyLineHeight
                             +MyBottomShadowHeight + MyBottomMarginHeight );

If you ever change the drawing, how likely is that you'll recall what separate numbers 67 or 42 were made up of? Any compiler worth its salt will fold the numeric constants and thus generate the same code for both of them. There is no reason to not go for readable code.

Closing words

Reading this, you may think I should just stop writing thoughtless or bad code instead of doing things like these to mask the issues. But the matter of the fact is: Everyone has a bad day, everyone makes a mistake. Particularly when you program in teams and you're programming all week long and there are deadlines when you have to ship, the likelihood of mistakes increases.

And even if you're not working in a team, remember the Zarra description of programming alone: You're programming in a team of three people. Past You, who was a moron, Present You, who is average, and Future You, who is a genius. You're already being annoyed by Past You's lack of skill, you don't want to make it any harder on Future You.

Following the rules in this article will make many bugs more obvious while you're debugging them, and will prevent many crashes from happening in the first place.

So, do you guys have any neat coding tricks to share that I forgot to mention?

Reader Comments: (RSS Feed)

Brian Webster writes: I actually go the opposite direction when it comes to using -autorelease. I try to use -release wherever I can, because it makes it a lot easier to debug objects that are getting releaseed one too many times. If you autorelease everything, this often results in your crash coming when the autorelease pool is drained, rather than the point in the code where you did the actual release. These kinds of bugs can be pretty messy to track down, whereas if it crashes immediately when you call -release, it gives you a stack trace directly where the extra release is occurring. I guess I also tend to be the type to have a single return at the end of my methods, so releasing at the end of the method is no big deal.

Michelle writes: Maybe the solution to the Weak Reference issue (e.g. NSTimer still having a reference to NSWindow after it is released) is to have NSTimer invoke a function in the NSWindowController instead?

Jeff writes: Tip: my mantra during initial setup is "Add an IBOutlet, Add an NSAssert()" to ensure that I remember to connect it up. As I recall, Apple disagree with you on using accessors in dealloc, specifically in the area of view management - it was something like "unless you nil the view member via the accessor it doesn't get cleaned up" - don't quote me on that though. The principle, however, was that if someone has subclassed you and started sending notifications, its their responsibility to have stop sending notifications. There shouldn't be anything a subclass can do that it can't undo. And as to #define MyPrettyColour "MyPrettyColour" I always make the symbol *different* to the value - that prevents people from misunderstanding and/or accidentally interchanging them. Initialising variables to nil is a fine thing except that it nukes the sometimes helpful 'using uninitialised variable' warning which nearly always points you at logic errors.

Jeff Johnson writes: Good advice. The correlate to "Don't Use Accessors in Constructors or Destructors" is "Always use accessors outside constructors or destructors". In other words, don't directly access ivars or attempt to memory-manage them outside the accessors. That often leads to trouble (leaks, crashes). Just implement the accessors as -(id)myStuff { return [[_myStuff retain] autorelease]; } and -(void)setMyStuff:(id)newStuff { [newStuff retain]; [_myStuff release]; _myStuff = newStuff }, then always use the accessors outside of init and dealloc, and you can't really go wrong.

Miklos Fazekas writes: You don't really explain why one should avoid accessors in constructor. The only thing is can think of is that you subclass might be overriding the accessors, and they are not prepared to run before the subclass constructor. But this is logic holds for normal methods, so you shouldn't call any methods on self from init at all?

Chris Suter writes: Regarding “NIL Everything That Isn't Bolted Down”: you don’t need to do what you suggest if you’ve got the appropriate compiler warning enabled, as everyone should do. The only issue with using the warning is that you only get the warnings if you’ve got optimisations turned on, i.e. it’s a Release build.

Uli Kusterer replies: ★ Brian, I can empathize with your sentiment. It is definitely something I've gone back and forth on, but in the end I've generally found many more leaks in code than I've found over-releases, so I'm opting for that approach for now. It is definitely a trade-off to make. Ideally, ObjC would have something like "auto-cleaning" variables, i.e. local variables that transparently do the retain/release dance like properties, then I'd head back in your direction. Michelle, yeah, in that particular case and many others, having the controller only schedule messages on itself is better. It also doesn't violate encapsulation of objects as much. I wrote that on Twitter, but apparently forgot to put it in the article. Thanks for reminding me, I'll add it back it! Jeff, would be curious, where do you put those asserts for IBOutlets. In -awakeFromNib? As to stopping to send notifications: Well, it's safe to e.g. do a performSelector: because it retains the target. The *real* bug here is the stale pointer. I agree, good code also cancels its notifications, but we're talking defensive coding here. This is the second line of defense after you've noticed an error has been made. Making the symbol different to the value is a neat idea, I'll have to ponder that. Jeff J., I fully agree. You should be going through accessors wherever you can, except in those exceptional cases I'm mentioning here. Miklos, yes, as I already explained in detail why it's bad in destructors, and I wrote that the same "applies to constructors in reverse", I thought everyone would be able to understand (like you did), that subclasses may override an accessor and expect some of their ivars to already be valid, while their constructor hasn't even run yet. And the immediate next chapter makes the point for calling other methods from the constructor. Chris, the compiler warning is nice in theory, but as I generally develop using debug builds and debugging is infuriating with optimizations turned on, the warning doesn't really help much. I do agree though, that (especially in the case of ObjC objects) nil can mask errors. Another option would be to init them to a known garbage value like libmallocdebug does (0x55555555).

Jeff writes: Yes, I put the asserts into awakeFromNib - after all, its missing connections in the nib that I'm trying to protect against. The thing about overly-defensive coding is that it tends to support "bad" practices in others. In my day job, we've had dozens of guys who say "you should always assume that the other guy does a bad job" and insist on "testing that the result is not nil, despite a successful return". The problem is that there is no rational place to stop - do you check every function that APPLE implements to make sure that it *really* returned a value, if its not documented to (occasionaly) return nil? Of course, this all comes from that feature of Objective-C that I hate the most, that messaging nil is legal - that is the most stupid decision in the history of computing. It made some lazy programmers lives a tiny bit easier, and opened up enormous bugholes for the rest of us. In rational languages where invoking methods against nil results in a crash which a debugger can catch, there is no argument as to whether its better or not to initialise values to nil. And I admit that my opinions are those of someone who regularly works in more than one language, and can't afford to think "if I'm working in C++, this is ok, if I'm working in Objective-C its bad". Good practices should (wherever possible) transcend mundane detail like the language you are using.

Uli Kusterer replies: ★ Jeff, fully agree that defensive coding can be overdone. Which is why I published the techniques above. These actually come from painful practical experience, and have in my experience actually improved matters. I don't agree at all that good coding practices should always transcend languages. Compare the way ObjC and C++ deal with exceptions, and cleaning up resources when they occur. C++ recommends stack objects like smart pointers, while ObjC recommends the autorelease pool. Different libraries and languages require different mindsets. Write ObjC like C++ (or vice versa), and you'll produce bad code. As to NIL-messaging: I like how a whole line can collapse into a no-op if one method in a chain returns NIL. It makes error handling much cleaner than ifs or try/catch, and is quite efficient as well. But I think particularly for IBOutlets, having a guaranteed crashing value would have been really good, esp. if ObjC auto-assigned that value to them.

Uli Kusterer replies: ★ Brian, I just remembered: Manually doing a retain/release means you'll have to explicitly catch exceptions if you don't want to leak on that occasion. That's another reason in favor of autorelease. Inner pools get cleaned up along with outer pools and such don't leak during an exception.

Jeff writes: Personally I see the nil chain being useful in a very small set of real world examples. The issue with cascading a whole bunch of method calls into one line is that if one of them somewhere in the chain fails, you can't tell which. Sure, the trivial x = [[[[myclass] alloc] init] autorelease] works ok but I don't see it in [[[[xmanager currentx] startTransaction] openEdit] addObject:something] because you'll nearly want those intermediate objects in a subsequent operation, and/or any of them can fail for a reason which you might need to communicate to the user in different ways. Forcing everyone to use a consistent set of Exception classes isn't workable because its not always your code in the chain, sometimes its 3rd party frameworks. In the case where they can be chained together trivially, there should probably already be a method that does the chaining for you. Like the [[...alloc]init] pair is in [...new]. As to the issue of Objective-C vs C++, what is your stance on whether it should be your responsibility to clean up an autorelease pool if someone under you @throws - ie, should you do NSAutoreleasePool *pool = [NSAutoreleasePool new]; @try { ... stuff ... } @finally { [pool drain]; } or do you rely on the hidden (but documented) behaviour that pools seem to autorelease themselves without you asking for it? Of course, garbage collection makes this particular example moot, but on an iPhone, its an issue since you can't afford to be lazy with your memory reclamation. Regardless of your opinion on this matter, I'm not trying to say that you should code Objective-C like C++ - but I think that the defensive measures you need to take in both are more alike than you'd think and that its better if the same mindset can be used in both cases. I'm biased, of course, because in my day job, our (multi-million line) application uses C, C++, C#, Python and our own home-grown OO-language, all interoperating, all calling each other, etc - I need strategies that are as consistently applicable as possible.

Uli Kusterer replies: ★ Jeff, the NIL chain works just fine with NSError return parameters and expressions stretched across multiple lines, as long as the message target is *always* the same variable that might be NIL. But yeah, it's still a fairly restricted set of circumstances. As to pools: Outer pools on the stack are documented to also flush any inner pools on top of them. There is no "magic" there. Only the bottom-most pool (e.g. in a thread) might need to be specially released.

Peter Hosey writes: “The problem with C is that local variables are not initialized to zero.” Not exactly. The problem is that a variable can (and must) exist before anything has ever been put in it. Try that in Python, for example: print x x = 42 You'll get an exception because the variable x doesn't exist yet. “There is a nice little example implementation of the Singleton design pattern on Apple's developer web site. Implement it.” Better yet, don't. I have a blog post analyzing the several ways that example is wrong, and presenting a much better one: http://boredzo.org/blog/archives/2009-06-17/doing-it-wrong

Uli Kusterer replies: ★ Not sure what point you're trying to make with regard to C not initializing to zero. But anyway, C is C, and as soon as a variable is declared, you want it to be NIL or NULL for safety's sake, because otherwise you don't notice if you forget to init something. Python may warn you, but that's really not of much help if you're stuck writing ObjC on a Mac. Sadly, your code is just as wrong as Apple's just in other places. I agree that the retain/release/autorelease/retainCount overrides are overkill and mask bugs elsewhere, but you're doing the sSharedInstance variable assignment when -init returns, not inside -init. This means that if anything in -init causes another call to +sharedInstance, you'll enter an endless loop of creating instances of your singleton. And if you think that can't happen, remind yourself that the main point we use singletons instead of classes with class methods is that you can subclass them, because the first instance of a singleton class or subclass "wins". A careless subclasser can easily cause such a loop by accident, even though at this point it would otherwise be perfectly safe.

Chris writes: I don't know how I feel about not using accessors in constructors and destructors. I think I read in the Apple doco that you should use them, because using property accessors even in constructors makes sure your objects are retained, otherwise you might forget. Besides which, it is the very fact that accessors might do other stuff which is the very reason why you probably want to use them. Yes sure, there'll be cases where this doesn't work because a method is expecting the object to be more fully constructed, then you inherit the class and get into trouble. But people inheriting from classes can *always* break the assumptions of the underlying class. The good thing in objective-c is you've got a lot of flexibility in even overriding the constructors of super-classes entirely to solve this stuff. The alternative seems to be to risk tons of bugs by trying to work around using the setters and work around doing too much in the constructor, to avoid a pretty rare case that you can deal with when it arises. If you override a setter and it is complicated and making lots of assumptions about what else is constructed, make liberal use of NSAssert to catch it quickly. Discussions of performSelector: and explicit retains in constructors and figuring out how to avoid calling stuff without repeating code, sounds like a cure worse than the disease. Besides which, objective-c only has constructors by convention, not by a language feature. You can think of your alloced object as fully constructed as long as your code can cope with nil values anywhere. init is just a convenience multi-value setter. Of course this is extreme, but I think the problem is case by case. If you use enough asserts, then you will catch any bad assumptions of inheriting classes quickly, and they will solve their problems quickly enough. I'm willing to be convinced otherwise, but I'm not sure about this one.

Peter Hosey writes: “… but you're doing the sSharedInstance variable assignment when -init returns…” No, I do no such thing.

Uli Kusterer replies: ★ Peter, in your case it's a static variable called sharedInstance, but yes, you're doing it. Well, your sample code is, anyway.

Peter Hosey writes: Yes, I'm assigning sharedInstance. No, I'm not doing it in -init.

Uli Kusterer replies: ★ Peter, and not doing it in init, but rather after init returns, is the problem with your sample code. It's one of the things that Apple's code does right, because it helps avoid lots of nasty bugs.

Peter Hosey writes: OK, now your complaint is a bit clearer. I'm guessing that the problem case you're thinking of is somebody allocking and initing an instance without going through +sharedInstance? Your solution is still wrong, just less wrong. If you're treating a singleton as a multipleton or regular class by allocking and initing it, you have a bug either way: You are misusing the class. The real right thing for a singleton to do would be to detect that -init's caller is not +sharedInstance and throw an exception. Even so, consistent wrong behavior is better than inconsistent wrong behavior, so I've applied the fix you suggested to the post. Thanks. I'm not sure when I'll get around to implementing the right behavior (which would probably use backtrace(3) and dladdr); too many other things to do. Patches welcome.

Uli Kusterer replies: ★ Peter, that's not what I'm talking about. The issue I'm thinking of is e.g.: I have a singleton that represents a USB device. In -init I register for IOKit device-plugged-in callbacks. In response to such a callback, I send an NSNotification. Trouble is, IOKit sends device-plugged-in messages right away on registration for all devices that are already plugged in. Now, if someone subscribed to this notification with NIL as the object, they could already be subscribed before they call +sharedInstance. Inside of their notification handler, they call +sharedInstance to get additional information. With your code, that would result in endless recursion, because -init has already been called, but has not returned yet. So when the notification handler calls +sharedInstance, the sharedInstance static variable is still NIL, so +sharedInstance will alloc/init a second singleton, which will in turn register for callbacks, which will send the messages for the plugged-in devices, which will call +sharedInstance, which will alloc/init a third object... Now this is one pathological case (a real-world example, though), but by doing the sharedInstance setting at the start of -init, you're rid of that whole class of problem. After all, the notification could be replaced with a mutator method or whatever.

Uli Kusterer replies: ★ By the way, could you elaborate what the benefit of mucking about with +initialize is? The code would be much simpler if you just did the work in +sharedInstance and/or -init. You have to call +sharedInstance to create the object, or if you call alloc/init that's a mistake (but anyway, it would set the static sharedInstance variable). All the special ifs/thens to detect if you were called from +initialize or not seem kinda unnecessarily error-prone.

Peter Hosey writes: “I have a singleton that represents a USB device.” What if I go out and buy a second one and plug them both in? There is no singleton implementation that can solve problems caused by making a singleton where it isn't appropriate. Not mine, not yours, not Apple's, not anyone's. > Now, if someone subscribed to this notification with NIL as the object, they could already be subscribed before they call +sharedInstance. Inside of their notification handler, they call +sharedInstance to get additional information. Why would they not use the object of the notification?

Peter Hosey writes: > By the way, could you elaborate what the benefit of mucking about with +initialize is? Don't just look at +initialize; look at the whole implementation. It makes it practically impossible for multiple instances to exist: The only way I can think of is to send alloc, but not init, before creating the shared instance. That does point out a bug, BTW: If you do that, then init the allocked instance after creating the shared one, init will return the allocked instance without initializing or releasing it. I've just fixed that. The other benefit is a bit of thread-safety, as pointed out in the comments, although I wouldn't call this class “thread-safe” in general, having not tested it in any such situation.

Uli Kusterer replies: ★ Peter, >What if I go out and buy a second one and plug them both in? That's beside the point, but for the sake of completeness: There are USB devices where having more than one on the same machine wouldn't make sense, or simply wouldn't work. Think of something that inputs uncompressed video. A USB 2.0 bus would probably be at capacity with one already. Also, the class represents a (type of) device. If you have e.g. an audio compressor, you may have one singleton representing all devices. If I plug in several, it would transparently distribute the load across all of them, maybe even GCD-like across applications. >Why would they not use the object of the notification? For completeness: Because, at this point, they don't know what the object is. If it's a singleton anyway, you should never get the same notification from any object but this one. So why not allow the added convenience? Yes, it's shoddy programming, but it's a fairly valid assumption, so I'd like to defend against it. Also, it's makes loose coupling possible: I can have one object that just subscribes for a notification, while another can actually cause the singleton to be created at a later time, e.g. after important setup work has been done. Glad we worked out an agreement about setting sharedInstance in -init instead of after init returns. It's great to have a solid, full example out there at http://boredzo.org/blog/archives/2009-06-17/doing-it-wrong

Comment on this article: Name: E-Mail: (not shown, hashed for Gravatar) Web Site URL: (optional) Comment: (plain text only) Please Enter the following word:

Or E-Mail Uli privately.