Re: NSUndoManager retain/release of arguments - ad infinitum
Re: NSUndoManager retain/release of arguments - ad infinitum
- Subject: Re: NSUndoManager retain/release of arguments - ad infinitum
- From: Graham Cox <email@hidden>
- Date: Tue, 11 Jan 2011 16:39:06 +1100
My first thought, before I go further, is that to undo a sort, you are doing things way too complicated. You only need to pass the sort descriptors to the undo manager, not a copy of the data. Presumably adding and removing items in the data itself is also undoable, so at any time the data on which undo will operate is consistent with the data you have - there is no reason to make a copy of it. That alone could significantly simplify your code, and simpler is less buggy.
(As an aside, the way I usually handle sorting is to have a property in my data model called -sortedData, which takes the raw data and applies the current sort descriptors to it. The resulting sorted array can be cached to avoid having to perform the sort repeatedly as long as the sort descriptors don't change. When they do change, I simply invalidate the cache (i.e. release it). The actual sort only takes place when a piece of sorted data is later requested, at which point the sorted data is cached again. The table view stores the sort descriptors for you, so you don't even need to store that as a property yourself. As suggested, to undo a sort, pass the old descriptors to the undo manager and when undo is invoked, it restores the old descriptors and once again invalidates the cache. This approach is super easy and efficient - you certainly don't need to pass the previously sorted copy of the data as an undo parameter).
On 11/01/2011, at 3:37 PM, John Bartleson wrote:
> [/*index into database*/ setItemArray:newArray]; // @property (readwrite, assign) NSMutableArray *itemArray;
Specifying 'assign' here is probably wrong. It is not retaining the assigned array, so you are stating you don't own it. Yet I suspect that you do want to own it. You're relying on the undo manager retaining it because you have neglected to do so. That's probably why when you do the expected and correct release, it crashes, because once the undo manager performs the undo, it will release the invocation that is retaining the array, which will dealloc it. Since you're not retaining it, it crashes because you have a stale reference.
That is entirely your fault, not something peculiar to the undo manager.
I recommend not using synthesised properties until you get the hang of writing your own manual getters and setters, which help you to develop the understanding of how these things should be written. Having said that, changing this from assign to retain will probably fix your issue.
I wonder if you have repeated the same error for other object properties which are not being retained? That would explain why you are having to rely on undo manager retaining your objects for you.
One other thing with regard to MM and Undo. Recall that targets are not retained, so the undo manager correctly does not retain the target. Thus, as with all such code (e.g. the very similar NSNotificationCenter situation) when your target is deallocated, it should purge itself from the undo manager using -removeAllActionsWithTarget: failure to do that will also crash later when someone tries to perform an undo on a no longer existing target.
--Graham
_______________________________________________
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