[Adium-devl] Review Please

[David Smith]

On Apr 21, 2009, at 3:21 AM, Evan Schoenberg wrote:

>
> 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

Excellent catch. Testing that locally now. :)

		David