[Development] API review for a new QDnsResolver class

[Jeremy Lainé]

Le Nov 9, 2011 à 10:14 AM, André Somers a écrit :

> 
> 
> On Wed, Nov 9, 2011 at 9:17 AM, Jeremy Lainé <jeremy.laine at m4x.org> wrote:
> On 11/08/2011 10:57 PM, Thiago Macieira wrote:
> > On Tuesday, 8 de November de 2011 19:40:13 Jeremy Lainé wrote:
> >> - the QNAM-style API seems to be OK
> > Correct, but all functions in QDnsResolver are static.
> >
> > That means they could go into QDnsReply and we could rename the class simply
> > QDns. It worked for Qt 3...
> >
> 
> The methods are not static, the QDnsResolver instance initially owns all QDnsReply objects
> it returns. It also owns the QThreadPool used to perform the lookups.
> Initially? What does that mean exactly? When does QDnsResolver stops owning the QDnsReply objects?


It means you can use QDnsReply::setParent() to take over ownership of the reply (like QNetworkReply).


>  
> 
> >> - I have implemented QDnsReply::abort() to cancel a lookup request
> > Good.
> >
> 
> I forgot to mention: you can delete the QDnsReply at any time, as both QDnsReply and
> QDnsResolverRunnable access QDnsReplyPrivate via a QSharedPointer.
> In that case, wouldn't it make more sense then to return a QDnsReply (so not a pointer)? Hmmm... but that is of course not possible because it derives from QObject, right?
> 

Correct, QDnsReply has both an abort() slot and a finished() signal. 


> 
> >> - Robin mentioned adding a static QDnsResolver::instance() method, does
> >> anyone else have an opinion on this?
> > No need if all functions are static.
> 
> And since they are not?
> As I argued before: I think they should be, as the class you showed up contains no actual data (from the user's perspective of it, anyway). It causes problems with having to keep the instance alive while the DNS request is running, even though the object itself can not really be interacted with anymore. It doesn't even provide API to know if it can be savely deleted, or if it is still in the background managing running requests. So, when can the API user savely get rid of the requester object? IMHO, it would make sense to have the methods on QDnsResolver be static, and let those static methods reference some private (singleton?) instance of the object that owns stuff like a threadpool and the QDnsReplies. That frees the user from having to care about the lifetime of the resolver object.  
> 


Currently, QDnsResolver can be deleted at any time without fear of a crash. However if there are outstanding requests, this will block until the QThreadPool has finished.

I agree that it looks as though QDnsResolver methods should be static, although it does once again raise the question of QDnsReply ownership since the replies can no longer be owned by the QDnsResolver. However, I am unsure about putting the methods in QDnsReply (even if it gets renamed to just "QDns"), I don't find it very descriptive in terms of API.

Jeremy

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20111109/6caa55e8/attachment.html>