Details
- Reviewers
davidedmundson ivan - Group Reviewers
Plasma - Commits
- R120:6b6ae6162457: Runners: Convert foreach to for
Diff Detail
- Repository
- R120 Plasma Workspace
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
runners/bookmarks/fetchsqlite.cpp | ||
---|---|---|
99–100 | This can be made much better by providing a custom range object to iterate from keyValueBegin() to keyValueEnd() - converting to std::map is quite inefficient. |
Something like this:
template <typename T> class asRangeImpl { public: asRange(const T &data) : m_data{data} { } auto begin() const { return m_data.keyValueBegin(); } auto end() const { return m_data.keyValueEnd(); } private: const T &m_data; }; template <typename T> class asRange(const T& map) { return asRangeImpl<T>(map); }
We don't need any custom code, it's a QMap, it has existing iterators already which can be used.
Yes, the wrapper just serves for keyval iterators to be usable with a range-based for loop. Quite useful if there are a lot of these. In this case, where it is only one loop I agree it might be an overkill.
Just a single nit-pick left :)
runners/bookmarks/fetchsqlite.cpp | ||
---|---|---|
101 | entry->first and entry->second |
I wish your wrapper was part of Qt directly to avoid boilerplate and give a simpler more idimatic way to get a key/value iteration.
Just a single nit-pick left :)
You suggestion does not work (or at least QtCreator does not allow me to write like you suggested).
entry here is a QKeyValueIterator that needs to be dereferenced using std::pair<Key, T> QKeyValueIterator::operator*() to get its underlining std::pair.
I hate Qt... defining operator* without operator-> ... ugh
I've approved the patch, sorry for the bad nitpick :)