On Wednesday, 19 June 2013 10:28:09 CEST, Pali Rohár wrote:
    AddressbookInterface(QObject * parent) : QObject(parent) {}
    virtual ~AddressbookInterface() {}

I wouldn't make these inline, but in the .cpp file. Perhaps it's not needed for 
QObject, but the destructor is typically the first virtual function being 
defined, and it is often used for RTTI purposes (dynamic_cast etc). This is 
probably a huge oversimplification, but the lesson I've learned is that virtual 
destructors shall never be inlined.

     * - Matches are case INsensitive
* - The match aligns to either Name or a...@mail.com, ie. "ja" or "jkt" match
     *   "Jan Kundrát <j...@gentoo.org>" but "gentoo" does not
* - Strings in the ignore list are NOT included in the return, despite they may match.
     * - The return can be empty.

Oops, these comments don't really match reality now. For jan.kund...@foo.example.org, the completion matches against "jan", "kundrat", "foo" and "example" IIRC. I'm not sure that these details belong to .h file describing the interface, though -- perhaps leaving the completion to be implementation-defined is the way to go to accomodate e.g. fast LDAP searches which might only work on a subset of fields, or plugins which can somehow look at other fields of the contact info they manage (a cmopany with domain "lbbw.com", but a contact with "Organization: LandesWhatEver Bank" should probably match on "Landes" even if it's not present in the e-mail address,...) etc.
virtual void startComplete(const QString &input, const QStringList &ignores = QStringList(), int max = -1) = 0;

I got confused when reading this -- to me, the name suggests that it's a signal saying that 
"the starting phase has completed". Perhaps "requestCompletion"?

    virtual void startPrettyNamesForAddress(const QString &email) = 0;

s/start/request/ again for the same reason?

* empty email and name strings means opening window for adding new contact

This is a hack, IMHO. I'd prefer to split these into:

openAddContactWindow(const QString &mailAddress, const QString &prettyName)

openEditContactWindow(const QString &mailAddress)

private:
    /**
* @short Return name and email in format "[Name <]a...@mail.com[>]" needed by method startComplete()
     */
static inline QString formatAddress(const QString &name, const QString &email) {
        if (name.isEmpty() || name == email) return email;
        else return name + " <" + email + '>';
    }

I don't see a point in having a *private* static method available in the public 
header of a plugin interface. Please remove this from the plugin API -- perhaps 
into some utility class.

    void completeResult(const QString &input, const QStringList &list);

completionAvailable? OK, you prefer ...Result, so what about completionResult?

    virtual AddressbookInterface * create(QObject * parent) = 0;

Nitpicking: "QObject *parent" to match the coding style of trojita.

Cheers,
Jan

--
Trojitá, a fast Qt IMAP e-mail client -- http://trojita.flaska.net/

Reply via email to