Review Request 120151: Cleanups in Host::categories (applets/systemtray)

David Edmundson david at davidedmundson.co.uk
Thu Sep 11 23:02:44 UTC 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120151/#review66301
-----------------------------------------------------------



applets/systemtray/plugin/host.cpp
<https://git.reviewboard.kde.org/r/120151/#comment46285>

    then you want to remove this.
    Otherwise your lookup may be faster but you're doing it twice.


I'm not convinced it'd actually be faster to hash things when you only have ~10 tasks than to just loop through them.

Also when optimising it's always worth profiling first, only bottlenecks should be optimised otherwise it's a waste of effort.

As far as I can tell this is never called:

david [01:00:01] ~/projects/kde5/src/kde/workspace/plasma-workspace/applets/systemtray (master)
$ git grep categories
plugin/host.cpp:    QStringList categories;
plugin/host.cpp:    emit categoriesChanged();
plugin/host.cpp:QStringList Host::categories() const
plugin/host.h:    Q_PROPERTY(QStringList categories READ categories NOTIFY categoriesChanged)
plugin/host.h:    QStringList categories() const;
plugin/host.h:    void categoriesChanged();

so we could just delete the whole method?

- David Edmundson


On Sept. 11, 2014, 10:48 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120151/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2014, 10:48 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> Use a QSet with O(1) lookup instead of a QList with O(n) lookup to
> store and get the already added categories.
> 
> Use a switch instead of an if statement.
> 
> 
> Diffs
> -----
> 
>   applets/systemtray/plugin/host.cpp f501ded 
> 
> Diff: https://git.reviewboard.kde.org/r/120151/diff/
> 
> 
> Testing
> -------
> 
> Still compiles
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140911/3c2d89be/attachment.html>


More information about the Plasma-devel mailing list