On Wed, Nov 9, 2011 at 11:26 AM, Jeremy Lainé <jeremy.la...@m4x.org> wrote:
> > Le Nov 9, 2011 à 10:14 AM, André Somers a écrit : > > > > On Wed, Nov 9, 2011 at 9:17 AM, Jeremy Lainé <jeremy.la...@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). > OK. I would like it more if that was more explicit in the API (also for QNAM), but ok. I am not a big fan of ::setParent() in such contexts, it doesn't feel natural to me. > > >> >> >> - 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. > QFuture solves this by having the QFuture itself be a value class, and have a QFutureWatcher that supplies the signals. Might be overkill though here, and is nicer if the two are combined IMO. > > > >> >> - 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. > I agree with you on the putting methods in QDnsReply (and calling it QDns). I prefer to keep the reply just that: an object representing a (future) reply to a query, not the query itself. On the ownership: It looks like you will need to have an object with some state somewhere anyway. Somebody has to own that QThreadPool, for instance. So, why not make that object the (first) parent then? Problem then is: who will cleanup the reply objects, and when will that happen, as the user has no control over the lifetime. I don't like having the delete of QDnsResolver block untill are requests are finished. Once again: returning a shared pointer to the QDnsReply object would solve that (the QDnsResolverPrivate class would keep a shared pointer around as long as the request is active), but it seems that is not a favoured solution. Would it be difficult to have QObject::connect support QSharedPointer<QObject> as the QObject* argument? Or rather, I guess, classes that overload operator->() that return a QObject*? Andre
_______________________________________________ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
