| ivan |
These two methods can be used to switch to the previous/next activity in
alphabetical order. They are exposed via the DBus interface. This
functionality can also be accessed by kactivities(-cli) and plasmashell, which
currently both implement their own separate versions of it.
@ivan mentioned being a bit wary of keeping a sorted activity list in
kactivitymanagerd, so I've opted to only retrieve and sort the list on demand
here.
| No Linters Available |
| No Unit Test Coverage |
| Buildable 13849 | |
| Build 13867: arc lint + arc unit |
| src/service/Activities.cpp | ||
|---|---|---|
| 243 | I don't like the fact that it constantly resorts the activities. The second problem is that ListActivities returns a list in one order, and this traverses activities in another order. If this will work by name, probably the List functions should as well. | |
| src/service/Activities.cpp | ||
|---|---|---|
| 243 | List probably returns a fairly randomized (per session) list, seeing how it's returning keys() from a QHash. Maybe I misunderstood you on IRC, I thought you'd rather not store a sorted list here. Instead of keeping a sorted cache though, maybe we should just keep the original list in a sorted order? | |
You are right, they are randomized.
And, yes, that is what I meant on IRC - that redundancy due to keeping a sorted list will likely lead to future issues, but I see no other way if we want to be consistent regarding listing and switching.
As it is required to have fast access by UUID, the hash needs to stay. So, I'd say we will need the redundancy. Maybe there is some multi-key sorted multi-map in boost - kamd already uses boost::flat_map, so this would be fine. :)
Keep an alphabetically sorted list of activities
Summary:
Instead of re-sorting the activity list every time previous-/nextActivity gets
called, maintain a sorted list.
Inserting/Removing/Updating a sorted list does not need to resort every time - removing is easy, adding a new item is std::lower_bound (a binary search), and updating is a combination of the two.
| src/service/Activities.cpp | ||
|---|---|---|
| 54 | You can use anonymous namespace for this (instead of static) or just make it a non-static function. It can be marked as inline, although the compiler will probably do that regardless of you saying so. You can rename it to something like nameBasedOrdering - better communicates what it does. | |
| 342 | You can just find the activity in the list, and remove it - the order for the rest will not change. | |
| src/service/Activities_p.h | ||
| 76 | QList -> QVector. QList is an evil and slow class :) | |
Addressed @ivan's change requests
Summary: