Patch for kabc resources

Cornelius Schumacher schumacher at kde.org
Wed Jan 7 00:02:02 GMT 2004


On Tuesday 06 January 2004 14:57, Bo Thorsen wrote:
>
> - finishes the implementation of the private pointer and moves the
> mAddressBook attribute to it. mAddrMap is not moved, since it's used
> directly by most subclasses

This is an unnecessary indirection. Please don't do it.

> - removes the writeConfig method that just called the superclass
> method (might not be a good idea, since this would make it impossible
> to introduce writing config items later - I need info on wether the
> existance of this method is by choice)

This looks rather dubious, indeed. It would be better to have a separate 
function for writing special config instead of requiring to call the 
implementation of the base class, anyway.

> - has default implementations of asyncLoad and asyncSave, since the
> resource subclasses I've seen all have this exact implementation.
> Code duplication is bad, so it might as well be in the superclass

Sounds good.

> - Removes the bogus doOpen and doLoad abstract methods. It does not
> make sense that these have default implementations in the kresources
> Resource class, and then this subclass forces kabc resources to
> reimplement the same default implementations

Agreed.

> I have tested that it compiles, but I want feedback on wether this
> should be committed or not. It's BIC, but since the resources were
> not part of 3.1, that shouldn't be a problem.

Please be very careful with changes of the HEAD branch. As far as I can 
see all the changes are more cosmetic, so I don't think we actually 
need them now. The asyncLoad/Save and the doOpen/Load parts look 
relatively safe, though, so if Tobias is sure they won't break anything 
I'm fine with them.

-- 
Cornelius Schumacher <schumacher at kde.org>




More information about the kde-core-devel mailing list