Patch for kabc resources

Bo Thorsen bo at sonofthor.dk
Wed Jan 7 07:16:34 GMT 2004


On Wednesday 07 January 2004 01:02, Cornelius Schumacher wrote:
> 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.

I disagree with this, but I also don't really care. I'll revert this part 
to the old way.

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

The reason for changing it now is the fact that it's BIC - because we 
would be stuck with the problems I have described here until 4.0.

I will check in the fixes to doOpen, doLoad, loadAsync, and saveAsync. 
Thanks for the reviews.

Bo.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: signature
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20040107/dbe5bd75/attachment.sig>


More information about the kde-core-devel mailing list