From 94641c510f496dbc50305cf627be09b1bda51bd0 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sat, 24 Sep 2022 14:06:56 +0200 Subject: [PATCH 1/9] core: create range.h header for range manupulation functions The moveInVector() function was defined in qthelper.h, even though it has nothing to do with Qt. Therefore, move it into its own header. Morover, since it is a very low-level function, use snake_case. And rename it to move_in_range(), because it does not only work on vectors, but any range with random-access iterators. Signed-off-by: Berthold Stoeger --- Subsurface-mobile.pro | 1 + core/CMakeLists.txt | 1 + core/qthelper.h | 25 ------------------------ core/range.h | 32 +++++++++++++++++++++++++++++++ profile-widget/profilewidget2.cpp | 7 ++++--- qt-models/divepicturemodel.cpp | 3 ++- qt-models/diveplannermodel.cpp | 3 ++- qt-models/divetripmodel.cpp | 3 ++- 8 files changed, 44 insertions(+), 31 deletions(-) create mode 100644 core/range.h diff --git a/Subsurface-mobile.pro b/Subsurface-mobile.pro index f1afbceaf..36db76407 100644 --- a/Subsurface-mobile.pro +++ b/Subsurface-mobile.pro @@ -206,6 +206,7 @@ HEADERS += \ core/pref.h \ core/profile.h \ core/qthelper.h \ + core/range.h \ core/save-html.h \ core/statistics.h \ core/units.h \ diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt index 54cafd7d6..83ad52eb9 100644 --- a/core/CMakeLists.txt +++ b/core/CMakeLists.txt @@ -147,6 +147,7 @@ set(SUBSURFACE_CORE_LIB_SRCS qt-init.cpp qthelper.cpp qthelper.h + range.h sample.c sample.h save-git.c diff --git a/core/qthelper.h b/core/qthelper.h index ddc0d3ac3..af73cc813 100644 --- a/core/qthelper.h +++ b/core/qthelper.h @@ -108,31 +108,6 @@ void uiNotification(const QString &msg); #define TITLE_OR_TEXT(_t, _m) _t, _m #endif -// Move a range in a vector to a different position. -// The parameters are given according to the usual STL-semantics: -// v: a container with STL-like random access iterator via std::begin(...) -// rangeBegin: index of first element -// rangeEnd: index one *past* last element -// destination: index to element before which the range will be moved -// Owing to std::begin() magic, this function works with STL-like containers: -// QVector v{ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; -// moveInVector(v, 1, 4, 6); -// as well as with C-style arrays: -// int array[10] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; -// moveInVector(array, 1, 4, 6); -// Both calls will have the following effect: -// Before: 0 1 2 3 4 5 6 7 8 9 -// After: 0 4 5 1 2 3 6 7 8 9 -// No sanitizing of the input arguments is performed. -template -void moveInVector(Vector &v, int rangeBegin, int rangeEnd, int destination) -{ - auto it = std::begin(v); - if (destination > rangeEnd) - std::rotate(it + rangeBegin, it + rangeEnd, it + destination); - else if (destination < rangeBegin) - std::rotate(it + destination, it + rangeBegin, it + rangeEnd); -} #endif // 3) Functions visible to C and C++ diff --git a/core/range.h b/core/range.h new file mode 100644 index 000000000..fa4f1465c --- /dev/null +++ b/core/range.h @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0 +// Helper functions for range manipulations +#ifndef RANGE_H +#define RANGE_H + +// Move a range in a vector to a different position. +// The parameters are given according to the usual STL-semantics: +// v: a container with STL-like random access iterator via std::begin(...) +// rangeBegin: index of first element +// rangeEnd: index one *past* last element +// destination: index to element before which the range will be moved +// Owing to std::begin() magic, this function works with STL-like containers: +// QVector v{ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; +// move_in_range(v, 1, 4, 6); +// as well as with C-style arrays: +// int array[10] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; +// move_in_range(array, 1, 4, 6); +// Both calls will have the following effect: +// Before: 0 1 2 3 4 5 6 7 8 9 +// After: 0 4 5 1 2 3 6 7 8 9 +// No sanitizing of the input arguments is performed. +template +void move_in_range(Range &v, int rangeBegin, int rangeEnd, int destination) +{ + auto it = std::begin(v); + if (destination > rangeEnd) + std::rotate(it + rangeBegin, it + rangeEnd, it + destination); + else if (destination < rangeBegin) + std::rotate(it + destination, it + rangeBegin, it + rangeEnd); +} + +#endif diff --git a/profile-widget/profilewidget2.cpp b/profile-widget/profilewidget2.cpp index 89d1b2840..492588fbe 100644 --- a/profile-widget/profilewidget2.cpp +++ b/profile-widget/profilewidget2.cpp @@ -5,6 +5,7 @@ #include "core/event.h" #include "core/subsurface-string.h" #include "core/qthelper.h" +#include "core/range.h" #include "core/settings/qPrefTechnicalDetails.h" #include "core/settings/qPrefPartialPressureGas.h" #include "profile-widget/diveeventitem.h" @@ -868,8 +869,8 @@ void ProfileWidget2::pointsRemoved(const QModelIndex &, int start, int end) void ProfileWidget2::pointsMoved(const QModelIndex &, int start, int end, const QModelIndex &, int row) { - moveInVector(handles, start, end + 1, row); - moveInVector(gases, start, end + 1, row); + move_in_range(handles, start, end + 1, row); + move_in_range(gases, start, end + 1, row); } void ProfileWidget2::repositionDiveHandlers() @@ -1317,7 +1318,7 @@ void ProfileWidget2::pictureOffsetChanged(dive *dIn, QString filename, offset_t // Move image from old to new position int oldIndex = oldPos - pictures.begin(); int newIndex = newPos - pictures.begin(); - moveInVector(pictures, oldIndex, oldIndex + 1, newIndex); + move_in_range(pictures, oldIndex, oldIndex + 1, newIndex); } else { // Case 1b): remove picture pictures.erase(oldPos); diff --git a/qt-models/divepicturemodel.cpp b/qt-models/divepicturemodel.cpp index af2813e78..867929911 100644 --- a/qt-models/divepicturemodel.cpp +++ b/qt-models/divepicturemodel.cpp @@ -5,6 +5,7 @@ #include "core/imagedownloader.h" #include "core/picture.h" #include "core/qthelper.h" +#include "core/range.h" #include "core/selection.h" #include "core/subsurface-qt/divelistnotifier.h" #include "commands/command.h" @@ -312,6 +313,6 @@ void DivePictureModel::pictureOffsetChanged(dive *d, const QString filenameIn, o if (oldIndex == newIndex || oldIndex + 1 == newIndex) return; beginMoveRows(QModelIndex(), oldIndex, oldIndex, QModelIndex(), newIndex); - moveInVector(pictures, oldIndex, oldIndex + 1, newIndex); + move_in_range(pictures, oldIndex, oldIndex + 1, newIndex); endMoveRows(); } diff --git a/qt-models/diveplannermodel.cpp b/qt-models/diveplannermodel.cpp index 17be91a29..058bc3821 100644 --- a/qt-models/diveplannermodel.cpp +++ b/qt-models/diveplannermodel.cpp @@ -7,6 +7,7 @@ #include "qt-models/models.h" #include "core/device.h" #include "core/qthelper.h" +#include "core/range.h" #include "core/sample.h" #include "core/settings/qPrefDivePlanner.h" #include "core/settings/qPrefUnit.h" @@ -877,7 +878,7 @@ void DivePlannerPointsModel::editStop(int row, divedatapoint newData) if (newRow != row && newRow != row + 1) { beginMoveRows(QModelIndex(), row, row, QModelIndex(), newRow); - moveInVector(divepoints, row, row + 1, newRow); + move_in_range(divepoints, row, row + 1, newRow); endMoveRows(); // Account for moving the row backwards in the array. diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp index 2f35367a3..a4dbbf25b 100644 --- a/qt-models/divetripmodel.cpp +++ b/qt-models/divetripmodel.cpp @@ -10,6 +10,7 @@ #include "core/string-format.h" #include "core/trip.h" #include "core/qthelper.h" +#include "core/range.h" #include "core/divesite.h" #include "core/picture.h" #include "core/subsurface-string.h" @@ -974,7 +975,7 @@ void DiveTripModelTree::topLevelChanged(int idx) // If index changed, move items if (newIdx != idx && newIdx != idx + 1) { beginMoveRows(QModelIndex(), idx, idx, QModelIndex(), newIdx); - moveInVector(items, idx, idx + 1, newIdx); + move_in_range(items, idx, idx + 1, newIdx); endMoveRows(); } From cea171ffd4e11b40ed8df2f5e52a9c283fcf4f48 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sat, 24 Sep 2022 19:08:05 +0200 Subject: [PATCH 2/9] core: implement an enumerating iterator In the printing-template code, we loop through a vector and then determine the index of the current element by searching the vector. This irks me. Since looping over a collection with an index is a rather common theme, implement an enumerating iterator that can be used as in: for (auto [idx, item]: enumerated_range(v)) { ... } For now, use it for the above vexing case. Convert other iterations of this theme later. Signed-off-by: Berthold Stoeger --- core/range.h | 53 ++++++++++++++++++++++++++++++++ desktop-widgets/printoptions.cpp | 5 +-- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/core/range.h b/core/range.h index fa4f1465c..59dbf090d 100644 --- a/core/range.h +++ b/core/range.h @@ -3,6 +3,9 @@ #ifndef RANGE_H #define RANGE_H +#include // for std::pair +#include // we need a declaration of std::begin() and std::end() + // Move a range in a vector to a different position. // The parameters are given according to the usual STL-semantics: // v: a container with STL-like random access iterator via std::begin(...) @@ -29,4 +32,54 @@ void move_in_range(Range &v, int rangeBegin, int rangeEnd, int destination) std::rotate(it + destination, it + rangeBegin, it + rangeEnd); } +// A rudimentary adaptor for looping over ranges with an index: +// for (auto [idx, item]: enumerated_range(v)) ... +// The index is a signed integer, since this is what we use more often. +template +class enumerated_range +{ + Range &base; +public: + using base_iterator = decltype(std::begin(std::declval())); + class iterator { + int idx; + base_iterator it; + public: + std::pair operator*() const + { + return { idx, *it }; + } + iterator &operator++() + { + ++idx; + ++it; + return *this; + } + iterator(int idx, base_iterator it) : idx(idx), it(it) + { + } + bool operator==(const iterator &it2) const + { + return it == it2.it; + } + bool operator!=(const iterator &it2) const + { + return it != it2.it; + } + }; + + iterator begin() + { + return iterator(0, std::begin(base)); + } + iterator end() + { + return iterator(-1, std::end(base)); + } + + enumerated_range(Range &base) : base(base) + { + } +}; + #endif diff --git a/desktop-widgets/printoptions.cpp b/desktop-widgets/printoptions.cpp index e36351443..88cf57c4f 100644 --- a/desktop-widgets/printoptions.cpp +++ b/desktop-widgets/printoptions.cpp @@ -3,6 +3,7 @@ #include "templateedit.h" #include "templatelayout.h" #include "core/qthelper.h" +#include "core/range.h" #include #include @@ -63,10 +64,10 @@ void PrintOptions::setupTemplates() currList.sort(); int current_index = 0; ui.printTemplate->clear(); - Q_FOREACH(const QString& theme, currList) { + for (auto [idx, theme]: enumerated_range(currList)) { // find the stored template in the list if (theme == storedTemplate || theme == lastImportExportTemplate) - current_index = currList.indexOf(theme); + current_index = idx; ui.printTemplate->addItem(theme.split('.')[0], theme); } ui.printTemplate->setCurrentIndex(current_index); From e2338fe7e912f1bbf87ee6757b10729de3f1de1b Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sat, 24 Sep 2022 19:27:54 +0200 Subject: [PATCH 3/9] core: use range-based for loops in filterconstraints This source file was looping over descriptors in a classical "for (int i = 0; i < size; ++i)" loop. However, the index is not really used, except for fetching the actual elements. Replace by range-based for loops. This prevents the potential error of using the wrong size. Signed-off-by: Berthold Stoeger --- core/filterconstraint.cpp | 73 ++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 44 deletions(-) diff --git a/core/filterconstraint.cpp b/core/filterconstraint.cpp index 75bfaed0a..e5d5e4a60 100644 --- a/core/filterconstraint.cpp +++ b/core/filterconstraint.cpp @@ -104,26 +104,11 @@ static const char *negate_description[2] { QT_TRANSLATE_NOOP("gettextFromC", "is not"), }; -static constexpr size_t type_descriptions_count() -{ - return std::size(type_descriptions); -} - -static constexpr size_t string_mode_descriptions_count() -{ - return std::size(string_mode_descriptions); -} - -static constexpr size_t range_mode_descriptions_count() -{ - return std::size(range_mode_descriptions); -} - static const type_description *get_type_description(enum filter_constraint_type type) { - for (size_t i = 0; i < type_descriptions_count(); ++i) { - if (type_descriptions[i].type == type) - return &type_descriptions[i]; + for (const auto &desc: type_descriptions) { + if (desc.type == type) + return &desc; } report_error("unknown filter constraint type: %d", type); return nullptr; @@ -131,9 +116,9 @@ static const type_description *get_type_description(enum filter_constraint_type static const string_mode_description *get_string_mode_description(enum filter_constraint_string_mode mode) { - for (size_t i = 0; i < string_mode_descriptions_count(); ++i) { - if (string_mode_descriptions[i].mode == mode) - return &string_mode_descriptions[i]; + for (const auto &desc: string_mode_descriptions) { + if (desc.mode == mode) + return &desc; } report_error("unknown filter constraint string mode: %d", mode); return nullptr; @@ -141,9 +126,9 @@ static const string_mode_description *get_string_mode_description(enum filter_co static const range_mode_description *get_range_mode_description(enum filter_constraint_range_mode mode) { - for (size_t i = 0; i < range_mode_descriptions_count(); ++i) { - if (range_mode_descriptions[i].mode == mode) - return &range_mode_descriptions[i]; + for (const auto &desc: range_mode_descriptions) { + if (desc.mode == mode) + return &desc; } report_error("unknown filter constraint range mode: %d", mode); return nullptr; @@ -151,9 +136,9 @@ static const range_mode_description *get_range_mode_description(enum filter_cons static enum filter_constraint_type filter_constraint_type_from_string(const char *s) { - for (size_t i = 0; i < type_descriptions_count(); ++i) { - if (same_string(type_descriptions[i].token, s)) - return type_descriptions[i].type; + for (const auto &desc: type_descriptions) { + if (same_string(desc.token, s)) + return desc.type; } report_error("unknown filter constraint type: %s", s); return FILTER_CONSTRAINT_DATE; @@ -161,9 +146,9 @@ static enum filter_constraint_type filter_constraint_type_from_string(const char static enum filter_constraint_string_mode filter_constraint_string_mode_from_string(const char *s) { - for (size_t i = 0; i < string_mode_descriptions_count(); ++i) { - if (same_string(string_mode_descriptions[i].token, s)) - return string_mode_descriptions[i].mode; + for (const auto &desc: string_mode_descriptions) { + if (same_string(desc.token, s)) + return desc.mode; } report_error("unknown filter constraint string mode: %s", s); return FILTER_CONSTRAINT_EXACT; @@ -171,9 +156,9 @@ static enum filter_constraint_string_mode filter_constraint_string_mode_from_str static enum filter_constraint_range_mode filter_constraint_range_mode_from_string(const char *s) { - for (size_t i = 0; i < range_mode_descriptions_count(); ++i) { - if (same_string(range_mode_descriptions[i].token, s)) - return range_mode_descriptions[i].mode; + for (const auto &desc: range_mode_descriptions) { + if (same_string(desc.token, s)) + return desc.mode; } report_error("unknown filter constraint range mode: %s", s); return FILTER_CONSTRAINT_EQUAL; @@ -217,21 +202,21 @@ extern "C" int filter_constraint_range_mode_to_index(enum filter_constraint_rang extern "C" enum filter_constraint_type filter_constraint_type_from_index(int index) { - if (index >= 0 && index < (int)type_descriptions_count()) + if (index >= 0 && index < (int)std::size(type_descriptions)) return type_descriptions[index].type; return (enum filter_constraint_type)-1; } extern "C" enum filter_constraint_string_mode filter_constraint_string_mode_from_index(int index) { - if (index >= 0 && index < (int)string_mode_descriptions_count()) + if (index >= 0 && index < (int)std::size(string_mode_descriptions)) return string_mode_descriptions[index].mode; return (enum filter_constraint_string_mode)-1; } extern "C" enum filter_constraint_range_mode filter_constraint_range_mode_from_index(int index) { - if (index >= 0 && index < (int)range_mode_descriptions_count()) + if (index >= 0 && index < (int)std::size(range_mode_descriptions)) return range_mode_descriptions[index].mode; return (enum filter_constraint_range_mode)-1; } @@ -347,32 +332,32 @@ static double base_to_display_unit(int i, enum filter_constraint_type type) QStringList filter_constraint_type_list_translated() { QStringList res; - for (size_t i = 0; i < type_descriptions_count(); ++i) - res.push_back(gettextFromC::tr(type_descriptions[i].text_ui)); + for (const auto &desc: type_descriptions) + res.push_back(gettextFromC::tr(desc.text_ui)); return res; } QStringList filter_constraint_string_mode_list_translated() { QStringList res; - for (size_t i = 0; i < string_mode_descriptions_count(); ++i) - res.push_back(gettextFromC::tr(string_mode_descriptions[i].text_ui)); + for (const auto &desc: string_mode_descriptions) + res.push_back(gettextFromC::tr(desc.text_ui)); return res; } QStringList filter_constraint_range_mode_list_translated() { QStringList res; - for (size_t i = 0; i < range_mode_descriptions_count(); ++i) - res.push_back(gettextFromC::tr(range_mode_descriptions[i].text_ui)); + for (const auto &desc: range_mode_descriptions) + res.push_back(gettextFromC::tr(desc.text_ui)); return res; } QStringList filter_constraint_range_mode_list_translated_date() { QStringList res; - for (size_t i = 0; i < range_mode_descriptions_count(); ++i) - res.push_back(gettextFromC::tr(range_mode_descriptions[i].text_ui_date)); + for (const auto &desc: range_mode_descriptions) + res.push_back(gettextFromC::tr(desc.text_ui_date)); return res; } From f407f5269a8033b40957faaae64eff0cfea5c6ce Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sat, 24 Sep 2022 19:36:42 +0200 Subject: [PATCH 4/9] cleanup: use std::size() instead of arithmetics Signed-off-by: Berthold Stoeger --- core/qthelper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/qthelper.cpp b/core/qthelper.cpp index 1682db44b..369d12bc3 100644 --- a/core/qthelper.cpp +++ b/core/qthelper.cpp @@ -676,7 +676,7 @@ QString get_water_type_string(int salinity) QStringList getWaterTypesAsString() { QStringList res; - res.reserve(std::end(waterTypes) - std::begin(waterTypes)); // Waiting for C++17's std::size() + res.reserve(std::size(waterTypes)); for (const char *t: waterTypes) res.push_back(gettextFromC::tr(t)); return res; From 727d519046f3fd15d491a9daad9ba3327e1ad31a Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sat, 24 Sep 2022 19:42:07 +0200 Subject: [PATCH 5/9] cleanup: use singleton pattern for getPrintingTemplatePath*() Function-local statics are initialized on first invocation. No point in extra logic. Signed-off-by: Berthold Stoeger --- core/qthelper.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/qthelper.cpp b/core/qthelper.cpp index 369d12bc3..24426d36c 100644 --- a/core/qthelper.cpp +++ b/core/qthelper.cpp @@ -719,17 +719,17 @@ static const char *printing_templates = "printing_templates"; QString getPrintingTemplatePathUser() { - static QString path = QString(); - if (path.isEmpty()) - path = QString(system_default_directory()) + QDir::separator() + QString(printing_templates); + // Function-local statics are initialized on first invocation + static QString path(QString(system_default_directory()) + + QDir::separator() + + QString(printing_templates)); return path; } QString getPrintingTemplatePathBundle() { - static QString path = QString(); - if (path.isEmpty()) - path = getSubsurfaceDataPath(printing_templates); + // Function-local statics are initialized on first invocation + static QString path(getSubsurfaceDataPath(printing_templates)); return path; } From b63073e203bf10621caac8f0aa2a405a8731abdf Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sun, 30 Oct 2022 21:24:13 +0100 Subject: [PATCH 6/9] cleanup: use range based loops in model There were a number of classical "for (i = 0; i < size; ++i)" loops. Replace them either by plain range based loops if the index is not used or by enumerated_range() loops. Signed-off-by: Berthold Stoeger --- qt-models/divepicturemodel.cpp | 4 ++-- qt-models/divetripmodel.cpp | 38 ++++++++++++++++++---------------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/qt-models/divepicturemodel.cpp b/qt-models/divepicturemodel.cpp index 867929911..bab837201 100644 --- a/qt-models/divepicturemodel.cpp +++ b/qt-models/divepicturemodel.cpp @@ -206,8 +206,8 @@ void DivePictureModel::picturesAdded(dive *d, QVector picsIn) // Convert the picture-data into our own format std::vector pics; pics.reserve(picsIn.size()); - for (int i = 0; i < picsIn.size(); ++i) - pics.push_back(PictureEntry(d, picsIn[i])); + for (const PictureObj &pic: picsIn) + pics.push_back(PictureEntry(d, pic)); // Insert batch-wise to avoid too many reloads pictures.reserve(pictures.size() + pics.size()); diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp index a4dbbf25b..39a8a0f62 100644 --- a/qt-models/divetripmodel.cpp +++ b/qt-models/divetripmodel.cpp @@ -206,8 +206,8 @@ static QPixmap &getPhotoIcon(int idx) if (!icons) { const IconMetrics &im = defaultIconMetrics(); icons = std::make_unique(std::size(icon_names)); - for (size_t i = 0; i < std::size(icon_names); ++i) - icons[i] = QIcon(icon_names[i]).pixmap(im.sz_small, im.sz_small); + for (auto [i, name]: enumerated_range(icon_names)) + icons[i] = QIcon(name).pixmap(im.sz_small, im.sz_small); } return icons[idx]; } @@ -1008,25 +1008,27 @@ void DiveTripModelTree::addDivesToTrip(int trip, const QVector &dives) int DiveTripModelTree::findTripIdx(const dive_trip *trip) const { - for (int i = 0; i < (int)items.size(); ++i) - if (items[i].d_or_t.trip == trip) + for (auto [i, item]: enumerated_range(items)) { + if (item.d_or_t.trip == trip) return i; + } return -1; } int DiveTripModelTree::findDiveIdx(const dive *d) const { - for (int i = 0; i < (int)items.size(); ++i) - if (items[i].isDive(d)) + for (auto [i, item]: enumerated_range(items)) { + if (item.isDive(d)) return i; + } return -1; } int DiveTripModelTree::findDiveInTrip(int tripIdx, const dive *d) const { const Item &item = items[tripIdx]; - for (int i = 0; i < (int)item.dives.size(); ++i) - if (item.dives[i] == d) + for (auto [i, dive]: enumerated_range(item.dives)) + if (dive == d) return i; return -1; } @@ -1034,8 +1036,8 @@ int DiveTripModelTree::findDiveInTrip(int tripIdx, const dive *d) const int DiveTripModelTree::findInsertionIndex(const dive_trip *trip) const { dive_or_trip d_or_t{ nullptr, (dive_trip *)trip }; - for (int i = 0; i < (int)items.size(); ++i) { - if (dive_or_trip_less_than(d_or_t, items[i].d_or_t)) + for (auto [i, item]: enumerated_range(items)) { + if (dive_or_trip_less_than(d_or_t, item.d_or_t)) return i; } return items.size(); @@ -1122,8 +1124,8 @@ static QVector visibleDives(const QVector &dives) #ifdef SUBSURFACE_MOBILE int DiveTripModelTree::tripInDirection(const struct dive *d, int direction) const { - for (int i = 0; i < (int)items.size(); ++i) { - if (items[i].d_or_t.dive == d || (items[i].d_or_t.trip && findDiveInTrip(i, d) != -1)) { + for (auto [i, item]: enumerated_range(items)) { + if (item.d_or_t.dive == d || (item.d_or_t.trip && findDiveInTrip(i, d) != -1)) { // now walk in the direction given to find a trip int offset = direction; while (i + offset >= 0 && i + offset < (int)items.size()) { @@ -1406,8 +1408,8 @@ void DiveTripModelTree::divesSelectedTrip(dive_trip *trip, const QVector // Since both lists are sorted, we can do this linearly. Perhaps a binary search // would be better? int j = 0; // Index in items array - for (int i = 0; i < dives.size(); ++i) { - while (j < (int)items.size() && !items[j].isDive(dives[i])) + for (struct dive *dive: dives) { + while (j < (int)items.size() && !items[j].isDive(dive)) ++j; if (j >= (int)items.size()) break; @@ -1427,8 +1429,8 @@ void DiveTripModelTree::divesSelectedTrip(dive_trip *trip, const QVector // would be better? int j = 0; // Index in items array const Item &entry = items[idx]; - for (int i = 0; i < dives.size(); ++i) { - while (j < (int)entry.dives.size() && entry.dives[j] != dives[i]) + for (struct dive *dive: dives) { + while (j < (int)entry.dives.size() && entry.dives[j] != dive) ++j; if (j >= (int)entry.dives.size()) break; @@ -1663,8 +1665,8 @@ void DiveTripModelList::divesSelected(const QVector &divesIn) // Since both lists are sorted, we can do this linearly. Perhaps a binary search // would be better? int j = 0; // Index in items array - for (int i = 0; i < dives.size(); ++i) { - while (j < (int)items.size() && items[j] != dives[i]) + for (struct dive *dive: dives) { + while (j < (int)items.size() && items[j] != dive) ++j; if (j >= (int)items.size()) break; From df5bf728f97067d1f5d6dd6cbf8867b3e152c6a6 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sun, 30 Oct 2022 21:31:38 +0100 Subject: [PATCH 7/9] cleanup: remove thumbnail conversion function The chances that their are still users of the old thumbnail format (i.e. all thumbnails saved in the hash file) are basically 0. If there are they will just get their thumbnails rebuilt when opening the individual dives. Signed-off-by: Berthold Stoeger --- core/qthelper.cpp | 43 +------------------------------------------ 1 file changed, 1 insertion(+), 42 deletions(-) diff --git a/core/qthelper.cpp b/core/qthelper.cpp index 24426d36c..62f3f5755 100644 --- a/core/qthelper.cpp +++ b/core/qthelper.cpp @@ -36,7 +36,6 @@ #include #include #include -#include // TODO: remove with convertThumbnails() #include #include #include @@ -1072,45 +1071,6 @@ extern "C" char *hashfile_name_string() return copy_qstring(hashfile_name()); } -// During a transition period, convert old thumbnail-hashes to individual files -// TODO: remove this code in due course -static void convertThumbnails(const QHash &thumbnails) -{ - if (thumbnails.empty()) - return; - // This is a singular occurrence, therefore translating the strings seems not worth it - QProgressDialog progress("Convert thumbnails...", "Abort", 0, thumbnails.size()); - progress.setWindowModality(Qt::WindowModal); - - int count = 0; - for (const QString &name: thumbnails.keys()) { - const QImage thumbnail = thumbnails[name]; - - if (thumbnail.isNull()) - continue; - - // This is duplicate code (see qt-models/divepicturemodel.cpp) - // Not a problem, since this routine will be removed in due course. - QString filename = thumbnailFileName(name); - if (filename.isEmpty()) - continue; - - QSaveFile file(filename); - if (!file.open(QIODevice::WriteOnly)) - return; - QDataStream stream(&file); - - quint32 type = MEDIATYPE_PICTURE; - stream << type; - stream << thumbnail; - file.commit(); - - progress.setValue(++count); - if (progress.wasCanceled()) - break; - } -} - // TODO: This is a temporary helper struct. Remove in due course with convertLocalFilename(). struct HashToFile { QByteArray hash; @@ -1129,7 +1089,7 @@ static void convertLocalFilename(const QHash &hashOf, const return; // Create a vector of hash/filename pairs and sort by hash. - // Elements can than be accessed with binary search. + // Elements can then be accessed with binary search. QHash canonicalFilenameByHash; QVector h2f; h2f.reserve(hashOf.size()); @@ -1166,7 +1126,6 @@ void read_hashes() stream >> localFilenameOf; locker.unlock(); hashfile.close(); - convertThumbnails(thumbnailCache); convertLocalFilename(hashOf, localFilenameByHash); } QMutexLocker locker(&hashOfMutex); From 691d9e86deed3c0e500f5d564d8134b091e22940 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Mon, 31 Oct 2022 19:35:15 +0100 Subject: [PATCH 8/9] cleanup: implement index_of() and index_of_if() generics Search the index of an item in a container. Compare by equality or a lambda. The lack of these have annoyed me for a long time. Return the index of the first found element or -1 if no element found. Currently, only supports random-access operators. Might be trivially changed for forward iterators. Signed-off-by: Berthold Stoeger --- core/range.h | 16 ++++++++++++++++ qt-models/divepicturemodel.cpp | 6 ++---- qt-models/divetripmodel.cpp | 20 +++++--------------- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/core/range.h b/core/range.h index 59dbf090d..561424079 100644 --- a/core/range.h +++ b/core/range.h @@ -82,4 +82,20 @@ public: } }; +// Find the index of an element in a range. Return -1 if not found +// Range must have a random access iterator. +template +int index_of(const Range &range, const Element &e) +{ + auto it = std::find(std::begin(range), std::end(range), e); + return it == std::end(range) ? -1 : it - std::begin(range); +} + +template +int index_of_if(const Range &range, Func f) +{ + auto it = std::find_if(std::begin(range), std::end(range), f); + return it == std::end(range) ? -1 : it - std::begin(range); +} + #endif diff --git a/qt-models/divepicturemodel.cpp b/qt-models/divepicturemodel.cpp index bab837201..9abd16192 100644 --- a/qt-models/divepicturemodel.cpp +++ b/qt-models/divepicturemodel.cpp @@ -242,10 +242,8 @@ int DivePictureModel::rowCount(const QModelIndex&) const int DivePictureModel::findPictureId(const std::string &filename) { - for (int i = 0; i < (int)pictures.size(); ++i) - if (pictures[i].filename == filename) - return i; - return -1; + return index_of_if(pictures, [&filename](const PictureEntry &p) + { return p.filename == filename; }); } static void addDurationToThumbnail(QImage &img, duration_t duration) diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp index 39a8a0f62..e55d620df 100644 --- a/qt-models/divetripmodel.cpp +++ b/qt-models/divetripmodel.cpp @@ -1008,29 +1008,19 @@ void DiveTripModelTree::addDivesToTrip(int trip, const QVector &dives) int DiveTripModelTree::findTripIdx(const dive_trip *trip) const { - for (auto [i, item]: enumerated_range(items)) { - if (item.d_or_t.trip == trip) - return i; - } - return -1; + return index_of_if(items, [trip] (const Item &item) + { return item.d_or_t.trip == trip; }); } int DiveTripModelTree::findDiveIdx(const dive *d) const { - for (auto [i, item]: enumerated_range(items)) { - if (item.isDive(d)) - return i; - } - return -1; + return index_of_if(items, [d] (const Item &item) + { return item.isDive(d); }); } int DiveTripModelTree::findDiveInTrip(int tripIdx, const dive *d) const { - const Item &item = items[tripIdx]; - for (auto [i, dive]: enumerated_range(item.dives)) - if (dive == d) - return i; - return -1; + return index_of(items[tripIdx].dives, d); } int DiveTripModelTree::findInsertionIndex(const dive_trip *trip) const From 4bbe5646a5af95d82759f10d2177b05701400bdc Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Tue, 1 Nov 2022 12:12:19 +0100 Subject: [PATCH 9/9] cleanup: remove unused declarations in planner.h Most of these declared non existing functions or pointers. One [get_gas_idx()] was only used in one source file and doesn't have to be globally accessible Signed-off-by: Berthold Stoeger --- core/planner.c | 2 +- core/planner.h | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/core/planner.c b/core/planner.c index 598e568bf..a4c93ba77 100644 --- a/core/planner.c +++ b/core/planner.c @@ -92,7 +92,7 @@ int get_cylinderid_at_time(struct dive *dive, struct divecomputer *dc, duration_ return cylinder_idx; } -int get_gasidx(struct dive *dive, struct gasmix mix) +static int get_gasidx(struct dive *dive, struct gasmix mix) { return find_best_gasmix_match(mix, &dive->cylinders); } diff --git a/core/planner.h b/core/planner.h index 6d0353db0..48d2bb33f 100644 --- a/core/planner.h +++ b/core/planner.h @@ -42,22 +42,13 @@ extern "C" { extern int validate_gas(const char *text, struct gasmix *gas); extern int validate_po2(const char *text, int *mbar_po2); -extern timestamp_t current_time_notz(void); -extern void set_last_stop(bool last_stop_6m); -extern void set_verbatim(bool verbatim); -extern void set_display_runtime(bool display); -extern void set_display_duration(bool display); -extern void set_display_transitions(bool display); extern int get_cylinderid_at_time(struct dive *dive, struct divecomputer *dc, duration_t time); -extern int get_gasidx(struct dive *dive, struct gasmix mix); extern bool diveplan_empty(struct diveplan *diveplan); extern void add_plan_to_notes(struct diveplan *diveplan, struct dive *dive, bool show_disclaimer, int error); extern const char *get_planner_disclaimer(); extern char *get_planner_disclaimer_formatted(); extern void free_dps(struct diveplan *diveplan); -extern struct dive *planned_dive; -extern char *cache_data; struct divedatapoint *plan_add_segment(struct diveplan *diveplan, int duration, int depth, int cylinderid, int po2, bool entered, enum divemode_t divemode); struct divedatapoint *create_dp(int time_incr, int depth, int cylinderid, int po2);