[PATCH] Make KDiskFreeSpace more convenient -> KJob-like usage

Sebastian Trüg strueg at mandriva.com
Wed Jul 23 15:55:08 BST 2008


On Wednesday 23 July 2008 15:13:21 David Faure wrote:
> On Wednesday 23 July 2008, Sebastian Trüg wrote:
> > Hi,
> >
> > please find attached a patch for KDiskFreeSpace. I found it to be very
> > inconvenient to use. You would have to connect to two signals of a
> > one-shot object. That is really not nice. So the patch transforms the
> > usage to something very similar to KJob: a finished signal which has the
> > KDiskFreeSpace as parameter. You then check for success and get the
> > actual values from the object. Afterwards it is deleted as before.
> > Oh, and it now uses byte values instead of Kib. I don't really see the
> > reason there. Also, we are in KIO, so I thought KIO::filesize_t was
> > appropriate.
>
> Agreed up to here.
>
> > BTW: a one-shot-object that you can create yourself? IMHO it should only
> > delete itself when used through the static method.
>
> I disagree. By default KJobs work this way too: you create them yourself
> and they delete themselves automatically when done (unless you call
> setAutoDelete(false)).

This little difference solves the problem:
1. you can call setAutoDelete(false)
2. You, normally, never create a KJob yourself, you let a method handle that.

This code is weird:

KDiskFreeSpace* df = new KDiskFreeSpace( parent ); // <- You even give it a 
parent!!!
df->readDf( ... );
// now what, I created it, so policy says that I delete it:
delete df;
// ----> crash!

Not good API.

> By inheritance, KIO jobs work this way too (but ok, you don't use new for
> them, you use a static method that calls new, like you did here... still if

exactly. And that makes all the difference.

> you create a custom kjob or kio job like we did in e.g. kmail then it will
> also have autodelete by default).

> > May I commit? BC is no problem, old API is marked deprecated (in
> > documentation that is).
>
> OK.

cool. :)




More information about the kde-core-devel mailing list