Re: [Bibdesk-develop] [Bibdesk-commit] SF.net SVN: bibdesk: [8925] trunk/bibdesk

[Christiaan H. <cmh...@gm...>]

On 29 Dec 2006, at 9:28 PM, Adam R. Maxwell wrote:

>
> On Dec 29, 2006, at 11:14, Christiaan Hofman wrote:
>
>>
>> On 29 Dec 2006, at 7:59 PM, Adam R. Maxwell wrote:
>>
>>> Just so I can have my obligatory grumble about these changes:  the
>>> async DO server requires you to pass all necessary setup parameters
>>> in
>>> the init... method, because the server thread setup is async.  Why
>>> isn't that parameter being used in this case?
>>>
>>
>> Sorry, indeed that should happen, just forgot to pass it in the
>> initializer. I already fixed that.
>
> saw that right after I sent the message :).
>
>>
>>> I like the formal protocol/server idea, and I think it cleaned up  
>>> the
>>> interface significantly from the group side.  I'm just not a huge  
>>> fan
>>> of initializing objects with dictionaries or using them as ivars,
>>> since it tends to moves type checking from the compiler to runtime
>>> exceptions.  And the controller still needs to create the  
>>> dictionary,
>>> which seems to violate encapsulation.  Is there a way we can improve
>>> this?
>>>
>>> -- arm
>>>
>>
>> We want to have a generic initializer, I think, otherwise the group
>> has to do a lot explicitly and know which information to pass and
>> how. Now only the sheetController needs to know about the info to
>> generate, which I think is already a big improvement, and makes the
>> code much cleaner.
>
> I agree, but in that case I don't think the sheet should check
> if(type == BDSKSearchGroupZoom); it should just pass all information
> and let the group decide what to do with it.  Or add setters to the
> group object so we get type checking.
>

See my other mail: they have to be passed all together, not one by one.

To have some alternative to type checking, we could "require" that  
the dictionary only contains strings (apart from the type perhaps).

>> Also using it as an ivar is to avoid the read-
>> write lock, which is already built in now. Or we have the problem
>> that the DO server connection is not yet ready. So it is mostly to
>> cleanup the code a lot.
>
> Well, that's why I was passing the required setup variables in the
> init method.  All subsequent setting was done on the server thread (I
> think...), in order to avoid the OSAtomic... functions and locking.
> Either way works.
>

However the server might not yet be initialized when reading the  
serverInfo, in principle it is not even guaranteed be setup when it  
is changed (but that would be really slow).

>> BTW, I just checked, and using ZOOM_resultset_records speeds
>> retrieval up considerably.
>
> Good find!  Probably because it does the ZOOM_resultset_retrieve for a
> batch instead of singly, so they're all in the cache.  I missed that
> function when looking through the API.
>
> adam

That's right, also the "documentation" is rather lacking.

Christiaan