Differential D28761 Diff 79878 src/ui/cryptoconfigmodule.cpp

Changeset View

Standalone View

View Options

src/ui/cryptoconfigmodule.cpp

Context not available.
44#include <utils/formatting.h> 44​#include <utils/formatting.h>
45#include <QIcon> 45​#include <QIcon>
46#include <QDialogButtonBox> 46​#include <QDialogButtonBox>
47​#include <QSet>
47 48​
48#include <QSpinBox> 49​#include <QSpinBox>
49 50​
Context not available.
161 addPage(w, configOK ? QString() : i18n("GpgConf Error")); 162​ addPage(w, configOK ? QString() : i18n("GpgConf Error"));
162 } 163​ }
163 164​
164 const QStringList components = config->componentList(); 165​ const QStringList components = sortComponentList(config->componentList());
165 for (QStringList::const_iterator it = components.begin(); it != components.end(); ++it) { 166​ for (QStringList::const_iterator it = components.begin(); it != components.end(); ++it) {
166 //qCDebug(KLEO_UI_LOG) <<"Component" << (*it).toLocal8Bit() <<":"; 167​ //qCDebug(KLEO_UI_LOG) <<"Component" << (*it).toLocal8Bit() <<":";
167 QGpgME::CryptoConfigComponent *comp = config->component(*it); 168​ QGpgME::CryptoConfigComponent *comp = config->component(*it);
Context not available.
227 } 228​ }
228} 229​}
229 230​
231​QStringList Kleo::CryptoConfigModule::sortComponentList(const QStringList &components) {
mlaurentUnsubmitted
Done

new line after ')'

mlaurent: new line after ')'
232​ // components sorting algorithm:
233​ // 1. components with hardcoded order - see below
234​ // 2. other components sorted alphabetically
235​ const QStringList order = {"gpg", "gpgsm", "gpg-agent", "dirmngr", "pinentry", "scdaemon"};
mlaurentUnsubmitted
Done

QStringLiteral(...) around each element

mlaurent: QStringLiteral(...) around each element
236​
dvratilUnsubmitted
Done

Make it static, there's no need to initialize this every time. It could also be something more efficient than QStringList, like std::array (since we know the size of the list beforehand).

dvratil: Make it `static`, there's no need to initialize this every time. It could also be something…
andreylegayevAuthorUnsubmitted
Not Done

Makes sense. Done.
I'm not writing on C++ every day and I'm curious is there difference: const static or static const in this particular place ?

andreylegayev: Makes sense. Done. I'm not writing on C++ every day and I'm curious is there difference…
dvratilUnsubmitted
Not Done

Technically there's no difference, the meaning is the same. But the are people who would argue with you endlessly about which way is the truly correct way of writing static const :)

dvratil: Technically there's no difference, the meaning is the same. But the are people who would argue…
237​ QSet<QString> input_set = QSet<QString>(components.constBegin(), components.constEnd());
238​ QSet<QString> order_set = QSet<QString>(order.begin(), order.end());
239​
240​ QStringList result;
241​
242​ // add components with predefined order
243​ for (const auto &item: order) {
244​ if (input_set.contains(item)) {
245​ result.append(item);
246​ }
247​ }
248​
mlaurentUnsubmitted
Done

(const auto &item : order)

mlaurent: (const auto &item : order)
249​ // add the rest of components with sorting
250​ QStringList others = (input_set - order_set).values();
251​ others.sort();
252​ result.append(others);
253​
254​ return result;
255​}
256​
230bool Kleo::CryptoConfigModule::hasError() const 257​bool Kleo::CryptoConfigModule::hasError() const
231{ 258​{
232 return mComponentGUIs.empty(); 259​ return mComponentGUIs.empty();
Context not available.
dvratilUnsubmitted
Not Done

You could just resolve the others list using another for loop and !result.contains() and get rid of all the QSets completely. The order list as well as the components list will always be short enough so that the two QSets (and the cost of constructing and subtracting them) will not be measurably faster than two for loops and a linear check.

dvratil: You could just resolve the `others` list using another `for` loop and `!result.contains()` and…
andreylegayevAuthorUnsubmitted
Not Done

You're right. I didn't think about small amount of items, tried to write universal code.
Added another for. Pls check new version.

andreylegayev: You're right. I didn't think about small amount of items, tried to write universal code. Added…