There’s no need to check if a variable is nil before release-ing it, because calling a method on a NULL variable is a no-op in Objective-C. And the runtime already has a super-fast check for this case. So adding an extra if test into your code will probably slow things down.
Doing a pointless ” ==?nil” test is bad for two reasons.
Firstly, it’s more code for no good reason. Even if it’s just one more line, It’s one more line to debug and test. It’s one more place where something could go wrong.
Secondly, it’s not idiomatic Cocoa-code, so it signals that something strange is going on. That’s a problem for whoever is reading the code, because they have to stop and look around more carefully to figure out why the pointless tests are happening.
In summary, do NOT do this:
– (void)dealloc;
{
if(baseImage) {
[baseImage release];
baseImage = nil;
}
[super dealloc];
}?
Do this:
– (void)dealloc;
{
[baseImage release];
baseImage = nil;
[super dealloc];
}
Not very defensive .. I’ve had pointers assigned incorrectly / over written by buffer overflows and then tried to release them ending up in a crash.
Generally relying on the weird send a message to nil is not a very safe approach.
Comment by Tim — June 8, 2011 @ 3:09 pm
No, the problem there was that you had a buffer overrun, blaming the crash on releasing corrupted memory is like blaming an airplane crash on the ground.
There’s no defensive measure you can take against a bug that stomps over your ivars at the level of dealing with your ivars.
Except that it’s not weird — it’s a consistent and intentional, feature of the language.
Comment by Vincent Gable — June 15, 2011 @ 10:48 pm
The baseImage = nil is also redundant. If dealloc is getting called, the memory holding that pointer is about to be released anyway. The only way I can imagine that would prevent a dereference into freed memory is if super’s dealloc somehow references that ivar (a sign of bad architecture) or there’s some horrible threading issue. You’d want it crashing in those cases to let you know something bad is happening.
Comment by dave — July 13, 2011 @ 5:42 pm
You’re right Dave. But I very strongly believe in keeping a contract with my ivars that either they are nil, or point to a valid object. In fact, I’d go so far as to say that it’s easier to habitually do redundant
= nil;assignments than to ever think about when they’re redundant. This is easier to do with @properties, becauseself.baseImage = nil;does both in one statement.Also, there is a not-quite-best-practice-but-not-pathological case where niling out freed ivars saves your bacon. Say you override some of your setters to do
[self.someOtherObject noteChanges];whenever their value changes. If you don’t setsomeOtherObjecttonilwhen you release it, then you have to be very careful about the order you set things tonil, or you have to directly release your ivars. This adds complexity and failure points that don’t exist ifsomeOtherObjectis defined to always to be a message-able variable.Comment by Vincent Gable — July 13, 2011 @ 7:34 pm