(reposted for formatting) On Aug 15, 2014, at 16:24 , Daryle Walker <dary...@mac.com> wrote:
> As I’m stripping out the NSDocument stuff, I realize that I need some sort of > global object to hold each window(-controller) as I create them, otherwise > ARC will purge them soon after creation. The application delegate seems like > a good option, as it’s my only other custom class and Apple uses it to store > the window in sample projects with a single window. I read a neat technique > at <http://eschatologist.net/blog/?p=189>, and I’m wondering if I would ever > need it and should switch to something easier. It’s the right idea, but the wrong implementation. > @property (nonatomic, copy) NSSet * windowControllers; > @property (nonatomic, readonly) NSMutableSet * mutableWindowControllers; Make the “windowControllers” property ‘readonly’ too. Although it’s feasible to allow the entire set to be changed via the setter, there are other ways to do this (e.g. removeAll + add), and you’re unlikely to need to do that anyway. The ‘copy’ property “pretends” that the implementation is something other than it is. > - (NSSet *)windowControllers > { > return [NSSet setWithSet:_windowControllers]; > } It’s simpler to ‘return _windowControllers;’. Or, better, use ‘@synthesize windowControllers = _mutableWindowControllers;' Since the return value is type ‘NSSet*’, the client can’t use the pointer to mutate the collection. (You can typically assume the client won’t do something hacky, like cast the pointer back to a NSMutableSet*. If you can’t assume that, you’ve got bigger problems.) > - (void)setWindowControllers:(NSSet *)windowControllers > { > [_windowControllers setSet:windowControllers]; > } > > You don’t need this, if you make the “windowControllers” property ‘readonly’, > as I recommended above. > > - (void)addWindowControllersObject:(PrBrowserController *)controller > { > [_windowControllers addObject:controller]; > } > > - (void)addWindowControllers:(NSSet *)controllers > { > [_windowControllers unionSet:controllers]; > } > > - (void)removeWindowControllersObject:(PrBrowserController *)controller > { > [_windowControllers removeObject:controller]; > } > > - (void)removeWindowControllers:(NSSet *)controllers > { > [_windowControllers minusSet:controllers]; > } These are correct, though you only need one of the ‘add…’ and one of the ‘remove…’ methods. Although it’s not wrong to implement both of each, it’s overkill for a set that’s not going to be very large. It’s typical to implement only the 1st and 3rd of these. (Keep in mind, you’re unlikely to want to add a *set* of window controllers at once.) Note that these are the KVC “mutable unordered accessors” described here: https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/KeyValueCoding/Articles/AccessorConventions.html This means it’s safe to expose these methods publicly. Or you can keep them private, and let clients use “mutableWindowControllers” for all their mutation needs. This is a matter of taste only. > - (void)intersectWindowControllers:(NSSet *)controllers > { > [_windowControllers intersectSet:controllers]; > } No, you don’t need this, or any other accessors. If you do everything else right, any client invocation of (say) ‘[myAppDelegate.mutableWindowControllers intersectSet: controllers];’ will get automatically translated into a combination of ‘add…’ and ‘remove…’ methods *and will be KVO-compliant*. Your code isn’t KVO compliant. > @synthesize mutableWindowControllers = _windowControllers; No, this is wrong and dangerous. You’re handing out a mutable reference to your private ivar, thus allowing clients to run rampant on your property’s integrity. Instead, you should implement it like this: - (NSMutableSet*) mutableWindowControllers { return [self mutableSetValueForKey: “windowControllers”]; } This returns a proxy object, so your underlying set stays private, and any mutation that a client applies to the proxy is actually applied to your underlying data via your accessors (the ‘add…’ and ‘remove…’ methods) *and* is KVO compliant. Finally, my preference is to use a NSArray metaphor for this sort of thing, rather than a NSSet metaphor. Yes, the collection of window controllers is unordered, but NSSet uses ‘isEqual:’ equality rather than pointer equality to distinguish between members. By default, it’s the same thing, but the reliance on *not* having a customized ‘isEqual:’ for your window controller classes is a hidden fragility in your implementation. Again, because you’re not going to have a very big collection of window controllers, performance and efficiency of the underlying (set vs array) representation is not a significant issue. _______________________________________________ Cocoa-dev mailing list (Cocoa-dev@lists.apple.com) 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: https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com This email sent to arch...@mail-archive.com
