[Adium-devl] Review Please

[Evan Schoenberg]

On Apr 21, 2009, at 6:38 AM, David Smith wrote:

>
> 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
>
> Still busted (blew up my account prefs too this time), getting tired  
> and angry, so going to sleep. Will investigate again in the morning.  
> The nightly has a safe build posted, and tip is safe. I think this  
> fix was necessary but not sufficient, and something else I messed up  
> is breaking things.

I'm getting set up on hg (haven't had a chance earlier in the week to  
do so) and will mess with it while you get your beauty rest :)

-Evan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://adium.im/pipermail/devel_adium.im/attachments/20090421/0d2f0c12/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/0d2f0c12/attachment.sig>