ivan added inline comments.
INLINE COMMENTS
> GtkEventSpy.cpp:68
> + QDateTime visited;
> + QList<Application> *applications;
> +
No need for this to be a pointer to a list. Make it just `QList<Application>
applications`.
> GtkEventSpy.cpp:70
> +
> + Bookmark(){
> + applications = new QList<Application>();
The constructor will not be needed once `applications` stops being a pointer.
> GtkEventSpy.cpp:76
> +
> +QString Bookmark::latestApplication() const {
> + Application current = applications->first();
`{` which starts a function should be on a new line (I don't care much about
this, but let's follow the KF5 style)
> GtkEventSpy.cpp:77-78
> +QString Bookmark::latestApplication() const {
> + Application current = applications->first();
> + for (const Application &app : qAsConst(*applications)) {
> + if (app.modified > current.modified) {
When you make `applications` not to be a pointer `qAsConst` will not be needed
as this is a `const` member function.
> GtkEventSpy.cpp:89
> +public:
> + BookmarkHandler(){
> + current = nullptr;
Replace with:
BookmarkHandler()
: current(nullptr)
{
}
> GtkEventSpy.cpp:91
> + current = nullptr;
> + marks = QList<Bookmark>();
> + }
No need for this - marks are already default-constructed.
> GtkEventSpy.cpp:113
> + if (qName == QStringLiteral("bookmark")) {
> + current = new Bookmark();
> + current->href = QUrl(attributes.value("href"));
No need for dynamic allocation. Make it a normal variable instead of a pointer.
> GtkEventSpy.cpp:185
> + reader.setErrorHandler(&bookmarkHandler);
> + QXmlInputSource *source = new QXmlInputSource(&file);
> +
No need for dynamic allocation. Make it a normal variable instead of a pointer.
> meven wrote in GtkEventSpy.cpp:143
> It is to just extract the executable name, we don't want to have an exploding
> number of initiatingAgent for every argument and parameter that might come
> through here.
I meant it will be a problem if someone decides to have a space in the
executable like `my\ aweomse\ binary`. But this should not be an issue at the
moment.
REPOSITORY
R161 KActivity Manager Service
REVISION DETAIL
https://phabricator.kde.org/D23112
To: meven, #frameworks, ivan
Cc: ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh,
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed,
jensreuterberg, abetts, sebas, apol, mart
