Changeset View
Standalone View
client.cpp
Show First 20 Lines • Show All 1234 Lines • ▼ Show 20 Line(s) | 1234 | #ifdef KWIN_BUILD_ACTIVITIES | |||
---|---|---|---|---|---|
1235 | if (!Activities::self()) { | 1235 | if (!Activities::self()) { | ||
1236 | return; | 1236 | return; | ||
1237 | } | 1237 | } | ||
1238 | QString joinedActivitiesList = newActivitiesList.join(QStringLiteral(",")); | 1238 | QString joinedActivitiesList = newActivitiesList.join(QStringLiteral(",")); | ||
1239 | joinedActivitiesList = rules()->checkActivity(joinedActivitiesList, false); | 1239 | joinedActivitiesList = rules()->checkActivity(joinedActivitiesList, false); | ||
1240 | newActivitiesList = joinedActivitiesList.split(u',', QString::SkipEmptyParts); | 1240 | newActivitiesList = joinedActivitiesList.split(u',', QString::SkipEmptyParts); | ||
1241 | 1241 | | |||
1242 | QStringList allActivities = Activities::self()->all(); | 1242 | QStringList allActivities = Activities::self()->all(); | ||
1243 | | ||||
1244 | auto it = newActivitiesList.begin(); | ||||
1245 | while (it != newActivitiesList.end()) { | ||||
graesslin: coding style | |||||
1246 | if (! allActivities.contains(*it)) { | ||||
1247 | it = newActivitiesList.erase(it); | ||||
this looks dangerous: your removing from the container you are iterating over. I have seen this crash to often. I'd suggest to use the iterator API and use erase on the container. graesslin: this looks dangerous: your removing from the container you are iterating over. I have seen this… | |||||
no it isn't. foreach() does a shallow copy and then any modifications (which there normally won't be) would do a full copy. davidedmundson: no it isn't.
foreach() does a shallow copy
and then any modifications (which there normally… | |||||
luebking: It's just about the most inefficient approach ;-) | |||||
Again no. No worse than ever passing a QString somewhere. davidedmundson: Again no.
It's incrementing a ref counter.
No worse than ever passing a QString somewhere.
| |||||
It is efficient in the common case, not efficient in the case where it actually deletes an activity from the list. Now, this should be rare enough for this not to matter. Personal reason for not liking this is that this is a special 'feature' of Q_FOREACH - when someone decides in the future to replace it with a range-for, it will lead to problems. At least, add a comment above regarding deletion in Q_FOREACH. ivan: It is efficient in the common case, not efficient in the case where it actually deletes an… | |||||
1248 | } else { | ||||
1249 | it++; | ||||
1250 | } | ||||
1251 | } | ||||
1252 | | ||||
1243 | if (// If we got the request to be on all activities explicitly | 1253 | if (// If we got the request to be on all activities explicitly | ||
1244 | newActivitiesList.isEmpty() || joinedActivitiesList == Activities::nullUuid() || | 1254 | newActivitiesList.isEmpty() || joinedActivitiesList == Activities::nullUuid() || | ||
1245 | // If we got a list of activities that covers all activities | 1255 | // If we got a list of activities that covers all activities | ||
1246 | (newActivitiesList.count() > 1 && newActivitiesList.count() == allActivities.count())) { | 1256 | (newActivitiesList.count() > 1 && newActivitiesList.count() == allActivities.count())) { | ||
1247 | 1257 | | |||
1248 | activityList.clear(); | 1258 | activityList.clear(); | ||
1249 | const QByteArray nullUuid = Activities::nullUuid().toUtf8(); | 1259 | const QByteArray nullUuid = Activities::nullUuid().toUtf8(); | ||
1250 | m_client.changeProperty(atoms->activities, XCB_ATOM_STRING, 8, nullUuid.length(), nullUuid.constData()); | 1260 | m_client.changeProperty(atoms->activities, XCB_ATOM_STRING, 8, nullUuid.length(), nullUuid.constData()); | ||
▲ Show 20 Lines • Show All 889 Lines • Show Last 20 Lines |
coding style