Re: Cocoa class extension best practice
Re: Cocoa class extension best practice
- Subject: Re: Cocoa class extension best practice
- From: Andy Lee <email@hidden>
- Date: Tue, 15 Oct 2013 17:05:06 -0400
On Oct 15, 2013, at 3:50 PM, Steve Mills <email@hidden> wrote:
> We have extended NSMenu so we could add some other methods. Many of the methods iterate over the itemArray like so:
>
> for(NSMenuItem* item in [self itemArray])
>
> Instruments shows that we're leaking NSArrays created when itemArray returns.
I would submit a bug report to Apple that itemArray leaks its return value. Should be easy to demonstrate with a tiny example app. I'm curious to verify the leak myself, but laziness is currently winning over curiosity. :) Not that I don't trust you, I just like to see for myself.
> I assume whoever wrote these methods was assuming that itemArray would simply return the internal NSArray and not make a copy. I guess I would make the same assumption since it's not a copy* method.
No assumption should be made about the implementation of itemArray, including whether it returns an internal value or makes a copy. When you're using manual retain/release, the memory management rules only say that you do not own the returned value and should not release it. At most you can say that itemArray is violating the rules by under-releasing the returned object.
> So my question is, what's the best way to write extension methods on an existing Cocoa class like this?
This question isn't really about class extensions. The issues apply to any code that calls itemArray, given that as far as you can tell, it leaks.
Out of curiosity, when you say you have "extended" NSMenu, do you mean you've subclassed it? That is what I would infer from your mention of class extensions. Is there some reason you don't add a category instead? I wonder because you only mentioned adding methods, not overriding them or adding ivars.
> One way would be to assign the itemArray result to a local variable and release it when done (ARC is not turned on in this project yet):
>
> NSArray* items = [self itemArray];
>
> for(NSMenuItem* item in items)
> blah;
>
> [items release];
Even if this worked (you mention that it didn't), it's not good to fix a memory management problem by breaking the rules. Besides trying to make two wrongs into a right, what happens when Apple fixes itemArray? Then you'd be over-releasing.
> Thoughts?
Assuming itemArray is definitely leaking, you're stuck choosing a workaround. One possibility is to go through different methods that (hopefully) don't leak:
NSInteger numItems = [self numberOfItems];
for (NSInteger itemIndex = 0; itemIndex < numItems; itemIndex++)
{
NSMenuItem *item = [self itemAtIndex:itemIndex];
// blah
}
You could also use the above logic to implement a non-leaking version of itemArray in a category or class extension:
- (NSArray *)nonLeakingItemArray
{
NSMutableArray *itemArray = [NSMutableArray array];
NSInteger numItems = [self numberOfItems];
for (NSInteger itemIndex = 0; itemIndex < numItems; itemIndex++)
{
[itemArray addObject:[self itemAtIndex:itemIndex]];
}
return itemArray;
}
Then instead of [self itemArray] you could use [self nonLeakingItemArray]. I would guess the cost of constructing the array yourself is totally negligible, but only you can know for sure by trying it.
--Andy
_______________________________________________
Cocoa-dev mailing list (email@hidden)
Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com
Help/Unsubscribe/Update your Subscription:
This email sent to email@hidden