Review request: kdelirc

[David Faure]

On Monday 20 April 2009, Michael Zanetti wrote:
> Hi all,
> 
> I have moved kdelirc from playground to kdereview. We would like to ship it 
> again with the kdeutils package for 4.3. Friedrich is already informed and 
> welcomes it.
> 
> It is fully ported to Qt4/KDE4 and it is basically the same program as before 
> altough we did a lot of refactoring. I think the code is much more readable 
> and maintainable as at KDE3 times. It is still not perfect but we are at a 
> point now where we think it is ready again to be used out there.
> 
> You can find it at kdereview/utils/kdelirc

The READMEs still mention DCOP.

There is funny syntax in the Modes class (which inherits from qmap) :
    Mode retMode = operator[](remote)[mode];
Why not value(remote)[mode]? You could use value() to save a lot of contains()
checks, too (currently, the lookup is done twice, contains and then [];
in non-const methods where contains isn't checked, like add() and erase(),
operator[] even risks doing unwanted insertions)

Modes::generateNulls says:
        if (!contains(*i) || !operator[](*i).contains("")) operator[](*i)[""] = Mode(*i, "");
        if (!theDefaults.contains(*i)) theDefaults[*i].isEmpty();
The first line could use some simplificiations, but the second line is
even wierder: it doesn't do anything? (it calls the isEmpty() method and discards the result,
well and it creates an entry in theDefaults, as a side effect).
Was this meant to be theDefaults.insert(*it, "") ?
(well or even QString() instead of "", everywhere).

The #include <KApplication> in arguments.h is unneeded.

ProfileServer::getAction:
        if (getProfileById(appId)->actions()[actionId]) {
            return getProfileById(appId)->actions()[actionId];
Use a local variable please, to do all this stuff only once.

Prototype::argumentCount() and getArguments() [the "get" isn't needed] should be const.

RemoteServer::theInstance is never deleted (would be useful for memory
leak checking), use K_GLOBAL_STATIC.

-- 
David Faure, faure at kde.org, sponsored by Qt Software @ Nokia to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).