Patch for kabc resources
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
> > 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 189 bytes
More information about the kde-core-devel