[Adium-devl] Review Please

[Evan Schoenberg]

On Apr 21, 2009, at 5:02 AM, David Smith wrote:

> 	I've checked and doublechecked http://hg.adiumx.com/adium/rev/6d66254ee722 
> , as well as tested it, but this code makes me nervous. If people  
> could take a look that'd be awesome.

I do agree with your commit that the code is convoluted; the  
performance benefit versus our original preferences implementation was  
huge, though.

This change:
	if ([object isKindOfClass:[AIAccount class]]) {
    123 				myGlobalPrefs = &accountPrefs;
    124 				myUsersOfGlobalPrefs = &usersOfAccountPrefs;
    125 				myTimerForSavingGlobalPrefs = &timer_savingOfAccountCache;
    126 				globalPrefsName = [@"AccountPrefs" retain];
    127 				
    128 			} else {
    129 				myGlobalPrefs = &objectPrefs;
    130 				myUsersOfGlobalPrefs = &usersOfObjectPrefs;
    131 				myTimerForSavingGlobalPrefs = &timer_savingOfObjectCache;
    132 				globalPrefsName = [@"ByObjectPrefs" retain];
    133 			}
    134 			//myGlobalPrefs *must* be non-nil before we attempt to  
@synchronize() on it
    135 			if (*myGlobalPrefs == nil)
    136 				*myGlobalPrefs = [[NSMutableDictionary alloc] init];

I think that we should also be setting &objectPrefs or &accountPrefs  
to this new dictionary such that the next container which is using  
that same global prefs gets the same object... and so that changes  
aren't lost if we end up needing to save in +  
(void)preferenceControllerWillClose after starting with a nil  
accountPrefs.

The rest looks good to me.

-Evan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://adium.im/pipermail/devel_adium.im/attachments/20090421/d1592fbd/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PGP.sig
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part
URL: <http://adium.im/pipermail/devel_adium.im/attachments/20090421/d1592fbd/attachment.sig>