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