[Development] API review for a new QDnsResolver class

[Andre Somers]

Op 9-11-2011 19:35, Jeremy Lainé schreef:
> On 11/09/2011 07:21 PM, Peter Hartmann wrote:
>> On 11/09/2011 06:43 PM, ext Jeremy Lainé wrote:
>>> (...)
>>> A/ static QDnsReply* QDnsReply::lookup(QDnsReply::Type, QString);
>>>
>>> pro: easy to connect to the QDnsReply's signal
>>> con: it's entirely up to the user to handle deletion. Judging by your comments above, I
>>> doubt you favor it?
>>>
>>> or
>>>
>>> B/ static QSharedPointer<QDnsReply>   QDnsReply::lookup(QDnsReply::Type, QString);
>>>
>>> pro: memory ownership is explicit
>>> con: how used are our users to manipulating QSharedPointer with respect to signals and
>>> such?
>> I rather favour option B (lessons learned from QNAM), because I think option A might
>> lead to undeleted replies, which is what we experienced in the case of
>> QNetworkReply *QNetworkAccessmanager::get(...).
>>
> OK, there is always another option:
>
> C/ you make QDnsReply's constructor public, and let the user manage the lifetime of the
> object. This is what the Q3Dns API looks like. What I don't like about Q3Dns's API is that
> it's unclear when the request is actually made.
>
You mean, like this?
//user code
QDnsReply* dnsReply = new QDnsReply(this);
QDnsResolver::lookup(dnsQuery, dnsReply);

Not a big fan...

Lets stick to either A or B. *I* prefer B, but that much should be clear 
by now. I also suggest we ask somebody with more template fu than I can 
wield to look into if it would be feasable to let this work:

QSharedPointer<QDnsReply> dnsReply = QDnsResolver::lookup(dnsQuery);
connect(dnsReply, SIGNAL(finished()), this, SLOT(dnsFinished())); 
//directly connect, instead of:
connect(dnsReply.data(), SIGNAL(finished()), this, SLOT(dnsFinished())); 
//currently needed connect statement

André