Review request: kdelirc
michael_zanetti at gmx.net
Tue Apr 21 19:04:31 BST 2009
On Tuesday 21 April 2009 12:03:17 David Faure wrote:
> 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))
> 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
> Was this meant to be theDefaults.insert(*it, "") ?
> (well or even QString() instead of "", everywhere).
I have fixed it in most places now.
> The #include <KApplication> in arguments.h is unneeded.
replaced this with #include <kdemacros.h>.
More information about the kde-core-devel