0

I see some code with

@property (nonatomic, readwrite, retain) id something;

And they synthesise it:

@synthesize something = something_;

And in the constructor:

self.something = @"HELLO!";

I assume that, the above line effectively retains that string.

But then, in their dealloc method, they do this:

[self setSomething:nil];

I guess that it is fine, because I imagine that when you set a property to nil, the old value is released. But then, I noticed that all the other classes they did had something like

[something release];

Instead, so I'm no longer sure. Are both ways correct?

Saturn
  • 17,888
  • 49
  • 145
  • 271

3 Answers3

2

Short answer: Use ARC. It takes care of this stuff for you. It's much less error-prone, and just as fast as manual reference counting.

Longer answer:

If you use retained properties, then yes, setting the property to nil is the correct thing to do.

like this:

self.something = nil;

That works because the setter for a retained property first releases the old value, then retains the new value and assigns it to the property's iVar. Since the new value is nil the retain does nothing.

If in your second example: 

[something release];

something is the iVar for a property, this code will cause a future crash if it is called from anywhere but in the code for the object's dealloc method. The reason is that this releases the object, but does not zero out the iVar. Later, when the object that has a something property is released, its dealloc method fires. The code in the dealloc method should attempt to release the object's retained properties. Sending release to an object that was already deallocated causes a crash.

In your case, you are asking about the code in a dealloc method. In dealloc, calling [something release] and setting the property to nil have the same result of releasing the object. Invoking the setter is probably safer, though, since custom setters sometimes have other code with additional "side effects." Since you're writing the dealloc method, you should be the author of the class, and should be aware of any special code in the setter method.

Now, if something is an instance variable, not a property, the correct thing to do is 

[something release]
something = nil;

EDITED 5 June 2014 to discuss the case of code in a dealloc method.

Duncan C
  • 128,072
  • 22
  • 173
  • 272
  • "In dealloc it will attempt to release it's retained properties." That's wrong. – pronvit Jun 05 '14 at 02:07
  • @mifki No, it is most definitely not wrong. In manual reference counting, you have to write your dealloc method to release all retained properties. If you don't do that, you leak. – Duncan C Jun 05 '14 at 11:35
  • Oh, I thought you meant _it_ will _automatically_ release retained properties. Also the questions itself is about `dealloc`, so in this particular case there will be no more "later, when the object is released". – pronvit Jun 05 '14 at 13:08
  • Oh dear. I missed that bit in the OPs question: "But then, **in their dealloc method**, they do this..." Time to edit my answer. – Duncan C Jun 05 '14 at 13:34
1

Both are correct, [self setSomething:nil]; will be better, when something is released to 0 retainCount then dealloc. This prevent using something from crash with BAD_EXE.

As mifki said, setter to be called if use [self setSomething:nil]; so this is depended on what have you done in setter method, a good setter should care about set value to nil, and deal with the case properly, and will not be undesired.

And even if setter method implement can't be cared to set to nil always, the better release style should be :

[something_ release], something_ = nil;  //this should be safely release always
simalone
  • 2,768
  • 1
  • 15
  • 20
  • Hi, mifki's answer says that `[something_ release]` is better instead. I dunno if either of you could clarify about this discrepancy. Thanks! – Saturn Jun 05 '14 at 02:18
  • @JustKidding Usually in `dealloc` you, well, just deallocate stuff. So there's no code that will possibly access released but not nilled properties. Also, if you use custom setters for properties that, for example, trigger repaint, recalculate some internal values or something like this, you don't want these actions to be performed from `dealloc`. – pronvit Jun 05 '14 at 02:29
  • @JustKidding I have edited my answer. Hope to help you. – simalone Jun 05 '14 at 02:50
  • *"A good setter should care about set value to nil"* - I imagine that the default setter, generated by `@synthesize`, does take care of that? – Saturn Jun 05 '14 at 04:12
  • @JustKidding generated by @synthesize like this: `if(something_ != newThing){ [something_ release]; something_ = [newThing retain]; }` It just like: `[something_ release], something_ = nil; ` – simalone Jun 05 '14 at 05:00
1

Better to use [something_ release]. This won't cause setter to be called, which otherwise could cause some actions to be performed that are undesired in dealloc.

pronvit
  • 4,169
  • 1
  • 18
  • 27