[Development] API review for a new QDnsResolver class

[André Somers]

On Thu, Nov 3, 2011 at 12:40 PM, Jeremy Lainé <jeremy.laine at m4x.org> wrote:

> Based on some initial feedback I received regarding DNS SRV support in Qt,
> I have refactored my proposed code and introduced a "QDnsResolver" class
> which I would like to submit for API review. The point of this class is to
> provide a QNAM-style asynchronous API to perform DNS lookups.
>
> The resolver object itself looks like:
>
> class Q_NETWORK_EXPORT QDnsResolver : public QObject
> {
>    Q_OBJECT
>
> public:
>    QDnsResolver(QObject *parent = 0);
>    ~QDnsResolver();
>
>    QDnsReply *lookupService(const QString &serviceName, const QString
> &domainName);
>

> private:
>    Q_DECLARE_PRIVATE(QDnsResolver)
> };
>
>
The more I think about it, the more I think it is important to fix this:
who is responsible for the lifetime of the QDnsReply object?

This API does not make that clear. I like the pattern in itself (also in
QNAM), but I do think it would be an improvement if we were to use a shared
pointer to the reply object. That at least makes clear who has ownership of
the object, and prevents memory leaks when people don't realize they are
supposed to delete the object.

Alternatively, perhaps a look at QFuture would help. QFuture is another way
results that are not yet ready are handled in Qt, but this time it is
returned as a value instead of as a pointer. It would be nice we could come
up with a single approach for these kinds of things and use that all over
the place...

Another concern is the question why you need a QDnsResolver object at all.
It only has a single method, and it does not seem to benefit from it being
a QObject at all at the moment, unless you plan to mimick the API of QNAM
and add signals and slots on the Resolver object itself. If not, then it
might as well just be a static method of the class, or just a method in a
namespace called QDnsResolver or something like that. Currently, the user
will have to instantiate the QDnsResolver object, and call the
lookupService method. It is not so clear what needs to happen to that
instance after that. Can it be savely deleted again while the request is
still running?


Andre'
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20111104/3b5633f6/attachment.html>