11

I know that discussions about coding styles tend to end in disaster and endless flame wars, but that’s not what I want to reach. During the last decade I mainly saw two different coding styles for dealloc methods in Objective-C. The first and most common one was to place dealloc at the bottom of the file. This is also the style Apple uses in the Xcode default templates. The logic behind this seems to be that dealloc is invoked when the end of the object approaches, so the end of the file seems to be a nice metaphor.

On the other hand a couple of people tend to put dealloc directly below the @synthesize directives. This has two major disadvantages in my opinion:

  1. The top of the file gets cluttered with boring code.
  2. It’s harder to find the essential parts in your class, you have to scroll down.

The huge advantage in my opinion is that you have a direct visual connection between properties and the corresponding release message.

The other thing is niling already released variables. While I don’t think that this is necessary, especially in object context where the whole variable gets desctructed after dealloc ends, I tend to also nil the variables. I’m used to do this for variables in function scope, so I’m just consistent with my coding style.

This is how most of my classes look like:

@implementation Bar

@synthesize foo;

- (void)dealloc
{
  [foo release], foo = nil;

  [super dealloc];
}

// Initializers and other methods…

I already mentioned a couple of pros and cons. What do you think about this topic? What is the coding style you use in dealloc and why? Are there other pros and cons I forgot to mention?

I don’t want to start a flame war here. I just want to know what style you use and if you have specific reasons for this or if this doesn’t matter for you in the end.

Rafael Bugajewski
  • 1,588
  • 3
  • 20
  • 36
  • Eventho you don't want people to start discussions and flames, it will happen to questions like this. – Younes Jan 25 '10 at 15:19

6 Answers6

16

I like to put the dealloc implementation right below the initializers. That way, when I add a new instance variable, I remember to release it right after I init it.

Also, I find it really helpful to use the #pragma mark directive to make it easier to browse the file. So I "group" the init and dealloc methods together under a heading called "initializers". When browsing the file, having those headings makes it much easier to find what you're looking for without being distracted by the dealloc method.

It might be boring code, but man is it important.

Alex
  • 26,711
  • 3
  • 53
  • 74
  • I wrap initializer(s) + dealloc in one #pragma mark - Object Lifecycle (akin to Apple templates' "View Lifecycle" for UIViewController) – Nicolas Miari Jul 08 '12 at 17:20
10

Don't set your ivar to nil in dealloc if you don't have a specific reason to. It serves no purpose and at best masks programmer errors that you would do better finding out about than hiding.

  • 1
    +1: By not setting the instance variables to `nil` and using good ol' `NSZombieEnabled`, I've caught a few logical errors over the years I never would have found otherwise. – Alex Feb 03 '10 at 07:57
8

My order:

  1. Syntheses and @dynamic directives (I started doing these at the top sometime in 2011; previously, they were in with accessor implementations)
  2. Class methods (+load, +initialize, +sharedFoo, others)
  3. Initializers
  4. dealloc
  5. finalize
  6. Custom accessor implementations
  7. Protocol conformance methods, grouped by protocol (usually with #pragma mark directives)
  8. Notification handler methods (usually declared in a class extension up at the top)
  9. Other methods (usually declared in a class extension up at the top)

Within the dealloc method:

  • Don't use accessor messages, implicit (property accesses) or explicit. Any impure custom accessor may not be safe to call on a partially-deallocated object. (The same goes for initializers.)
  • Don't set ivars to nil. The object is partially deallocated; why are you still sending it messages? (If you aren't, then nothing is looking at the values of the ivars.)
  • (If it were somehow appropriate to set ivars to nil) Don't abuse the comma operator. An expression like [foo release], foo = nil mixes the types (first void from the message expression, then id from the assignment expression). These are separate statements; write them as such.
  •  
    [super dealloc] is always last and always has an empty line above it, emphasizing its presence.

Of course, I also have “Treat Warnings as Errors” turned on, so if I forget [super dealloc], I break my build.

Peter Hosey
  • 93,914
  • 14
  • 203
  • 366
  • 1
    These are a lot of good tips for improving an Objective-C coding style. Thanks for sharing all of them. I know that it’s pointless to set the pointers to `nil` after releasing them in `dealloc`. I just do it, because I’m used to do it in a method context. I also use a blank line above `[super dealloc]` to emphasize the importance. An interesting thing is that you synthesize your properties in between of methods. Do you have a specific reason for this? – Rafael Bugajewski Jan 27 '10 at 11:11
  • 1
    It's a historical artifact: Before properties, all accessors were custom, and I didn't always declare them in the header. Thus, the definition would be the first the compiler heard of them, so I couldn't use them any earlier or the compiler would rightly complain about it. Thus, these undeclared accessors had to be defined before anything that might use them. Now I synthesize almost all of my accessors, but my choice of placement lives on. – Peter Hosey Jan 27 '10 at 11:17
  • Nowadays, I synthesize at the top of the implementation, just below any explicit instance variables. This way, all of the instance variables—explicit and synthesized—are together, and available throughout the class, including in initializers and `dealloc`. – Peter Hosey Jan 11 '12 at 23:33
  • I don't think setting an ivar to nil directly counts as "sending the object a message". Also, the object is not even 'partially deallocated' until you call super dealloc (unless you count releasing object ivars as 'partially reallocating the object'. The hard stuff happens in -[NSObject dealloc], not before I believe. – Nicolas Miari Jul 08 '12 at 17:24
  • @ranReloaded: All true, but beside the point. `dealloc` is called so that the object can release everything it owns and then deallocate itself. There is no reason (under MRC) to set an ivar to `nil` unless you expect the object to receive a message that might cause it to try to use an object that it has already released—but why should that happen while the object has no owners and is about to deallocate itself? There is no good reason; that happening indicates that you have memory-management problems. – Peter Hosey Jul 08 '12 at 20:25
  • That's correct. Also, if your object receives messages in the middle of dealloc, I think you also have Concurrency problems. – Nicolas Miari Jul 09 '12 at 00:16
  • @ranReloaded: That's one way it can happen. The other way is that you're sending yourself messages from `dealloc`, and either those methods or methods that they call attempt to use objects in the ivars. – Peter Hosey Jul 09 '12 at 01:22
5

I put my dealloc at the top, just under the @synthesize directives. It is a little clunky, and boring code, but oh-so-important code, so it gets top billing. Also, being able to compare between the properties and the -releases is vital.

Ben Gottlieb
  • 84,498
  • 22
  • 171
  • 170
3

I put it at the bottom. This allows me to simply hit end and go to it when I add something that needs deallocating. I also don't want it around my property synthesizers, because this is deceptive. Not everything I dealloc necessarily has a synthesized accessor attached to it. Heck, it isn't even necessarily all in the initializer. If I try to use a shortcut that way, I'm likely to mess it up.

Chuck
  • 222,660
  • 29
  • 289
  • 383
-1
- (id)init{
   self = [super init];
   if( self ) {
      someVar = [[NSMutableArray alloc] init];
      // something like the following shouldn't be released:
      someString = [NSString stringWithFormat:@"ANumber: %d",10];
   }
   return self;

   - (void)dealloc{
       [someVar release]; someVar = nil;
       [super dealloc];
   }

that's the way i do it :)

Antwan van Houdt
  • 6,931
  • 1
  • 27
  • 50
  • 4
    I'm pretty sure stringWithFormat returns an autoreleased object. You need to retain it in init and release in dealloc. Also, your braces don't balance... – walkytalky Jan 25 '10 at 19:54
  • read please, something like the following shouldn't be released as I state in my comment in the code, What i meant with that is that the object I am using there is autoreleased. – Antwan van Houdt Mar 03 '10 at 20:52
  • Sending messages to self inside dealloc is, at the very least very bad practice! – Nicolas Miari Jul 09 '12 at 06:27
  • @ranReloaded educate yourself please – Antwan van Houdt Jul 15 '12 at 17:05
  • Thanks for the suggestion, I'm doing it all the time. – Nicolas Miari Jul 16 '12 at 03:56
  • Still, even though the actual deallocation should not happen until -[NSObject dealloc] runs (usually the 'final' stage of [super dealloc]), sending messages to an object in the course of being deallocated does not 'feel' right (this question was about what does everybody do, wasn't it?). Unless it's a method dedicated to clean up, to keep the body of -dealloc small. In any case, this is _my taste_. – Nicolas Miari Jul 16 '12 at 11:40