From 5c7cfb1057cb100fb084f20d919d4d0257a0be1a Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sun, 2 Jun 2024 17:06:18 +0200 Subject: [PATCH] core: replace list of dives in trip by std::vector<> The dive_table will be converted into a table of owning pointers. Since the trip has only non-owning pointers to dives, turn its dive_table into an std::vector. Add a helper functions to add/remove items in a sorted list. These could be used elsewhere. Signed-off-by: Berthold Stoeger --- commands/command_divelist.cpp | 14 +++---- commands/command_edit.cpp | 2 +- core/divelist.cpp | 66 +++++++++++++++++--------------- core/divelog.cpp | 2 +- core/range.h | 19 ++++++++- core/save-html.cpp | 4 +- core/selection.cpp | 2 +- core/string-format.cpp | 2 +- core/trip.cpp | 54 +++++++++++++------------- core/trip.h | 4 +- desktop-widgets/divelistview.cpp | 8 ++-- qt-models/divetripmodel.cpp | 16 ++++---- 12 files changed, 107 insertions(+), 86 deletions(-) diff --git a/commands/command_divelist.cpp b/commands/command_divelist.cpp index 9dabf4f80..ec73a2a73 100644 --- a/commands/command_divelist.cpp +++ b/commands/command_divelist.cpp @@ -40,7 +40,7 @@ DiveToAdd DiveListBase::removeDive(struct dive *d, std::vectordive_site) diveSiteCountChanged(d->dive_site); res.site = unregister_dive_from_dive_site(d); - if (res.trip && res.trip->dives.nr == 0) { + if (res.trip && res.trip->dives.empty()) { divelog.trips->sort(); // Removal of dives has changed order of trips! (TODO: remove this) auto trip = remove_trip_from_backend(res.trip); // Remove trip from backend tripsToAdd.push_back(std::move(trip)); // Take ownership of trip @@ -273,7 +273,7 @@ static std::unique_ptr moveDiveToTrip(DiveToTrip &diveToTrip) // Remove dive from trip - if this is the last dive in the trip, remove the whole trip. dive_trip *trip = unregister_dive_from_trip(diveToTrip.dive); - if (trip && trip->dives.nr == 0) + if (trip && trip->dives.empty()) res = remove_trip_from_backend(trip); // Remove trip from backend // Store old trip and get new trip we should associate this dive with @@ -644,7 +644,7 @@ void ShiftTime::redoit() sort_dive_table(divelog.dives.get()); divelog.trips->sort(); for (dive_trip *trip: trips) - sort_dive_table(&trip->dives); // Keep the trip-table in order + trip->sort_dives(); // Send signals QVector dives = stdToQt(diveList); @@ -794,10 +794,10 @@ MergeTrips::MergeTrips(dive_trip *trip1, dive_trip *trip2) if (trip1 == trip2) return; std::unique_ptr newTrip = combine_trips(trip1, trip2); - for (int i = 0; i < trip1->dives.nr; ++i) - divesToMove.divesToMove.push_back( { trip1->dives.dives[i], newTrip.get() } ); - for (int i = 0; i < trip2->dives.nr; ++i) - divesToMove.divesToMove.push_back( { trip2->dives.dives[i], newTrip.get() } ); + for (dive *d: trip1->dives) + divesToMove.divesToMove.push_back( { d, newTrip.get() } ); + for (dive *d: trip2->dives) + divesToMove.divesToMove.push_back( { d, newTrip.get() } ); divesToMove.tripsToAdd.push_back(std::move(newTrip)); } diff --git a/commands/command_edit.cpp b/commands/command_edit.cpp index f7eec4aec..6a9ecc78a 100644 --- a/commands/command_edit.cpp +++ b/commands/command_edit.cpp @@ -1448,7 +1448,7 @@ void EditDive::exchangeDives() if (newDive->divetrip != oldDive->divetrip) qWarning("Command::EditDive::redo(): This command does not support moving between trips!"); if (oldDive->divetrip) - sort_dive_table(&newDive->divetrip->dives); // Keep the trip-table in order + newDive->divetrip->sort_dives(); // Keep the trip-table in order emit diveListNotifier.divesTimeChanged(delta, dives); } diff --git a/core/divelist.cpp b/core/divelist.cpp index 290509b97..937da49ed 100644 --- a/core/divelist.cpp +++ b/core/divelist.cpp @@ -814,11 +814,10 @@ static void merge_imported_dives(struct dive_table *table) * table. On failure everything stays unchanged. * If "prefer_imported" is true, use data of the new dive. */ -static bool try_to_merge_into(struct dive *dive_to_add, int idx, struct dive_table *table, bool prefer_imported, +static bool try_to_merge_into(struct dive *dive_to_add, struct dive *old_dive, bool prefer_imported, /* output parameters: */ struct dive_table *dives_to_add, struct dive_table *dives_to_remove) { - struct dive *old_dive = table->dives[idx]; struct dive *merged = try_to_merge(old_dive, dive_to_add, prefer_imported); if (!merged) return false; @@ -848,15 +847,13 @@ static bool dive_is_after_last(struct dive *d) * last dive of the global dive list (i.e. the sequence will change). * The integer pointed to by "num_merged" will be increased for every * merged dive that is added to "dives_to_add" */ -static bool merge_dive_tables(struct dive_table *dives_from, struct dive_table *delete_from, - struct dive_table *dives_to, +static bool merge_dive_tables(const std::vector &dives_from, struct dive_table *delete_from, + const std::vector &dives_to, bool prefer_imported, struct dive_trip *trip, /* output parameters: */ struct dive_table *dives_to_add, struct dive_table *dives_to_remove, int *num_merged) { - int i, j; - int last_merged_into = -1; bool sequence_changed = false; /* Merge newly imported dives into the dive table. @@ -868,15 +865,13 @@ static bool merge_dive_tables(struct dive_table *dives_from, struct dive_table * * - New dive "connects" two old dives (turn three into one). * - New dive can not be merged into adjacent but some further dive. */ - j = 0; /* Index in dives_to */ - for (i = 0; i < dives_from->nr; i++) { - struct dive *dive_to_add = dives_from->dives[i]; - - if (delete_from) - remove_dive(dive_to_add, delete_from); + size_t j = 0; /* Index in dives_to */ + size_t last_merged_into = std::string::npos; + for (auto dive_to_add: dives_from) { + remove_dive(dive_to_add, delete_from); /* Find insertion point. */ - while (j < dives_to->nr && dive_less_than(dives_to->dives[j], dive_to_add)) + while (j < dives_to.size() && dive_less_than(dives_to[j], dive_to_add)) j++; /* Try to merge into previous dive. @@ -885,9 +880,9 @@ static bool merge_dive_tables(struct dive_table *dives_from, struct dive_table * * In principle that shouldn't happen as all dives that compare equal * by is_same_dive() were already merged, and is_same_dive() should be * transitive. But let's just go *completely* sure for the odd corner-case. */ - if (j > 0 && j - 1 > last_merged_into && - dive_endtime(dives_to->dives[j - 1]) > dive_to_add->when) { - if (try_to_merge_into(dive_to_add, j - 1, dives_to, prefer_imported, + if (j > 0 && (last_merged_into == std::string::npos || j > last_merged_into + 1) && + dive_endtime(dives_to[j - 1]) > dive_to_add->when) { + if (try_to_merge_into(dive_to_add, dives_to[j - 1], prefer_imported, dives_to_add, dives_to_remove)) { delete dive_to_add; last_merged_into = j - 1; @@ -898,9 +893,9 @@ static bool merge_dive_tables(struct dive_table *dives_from, struct dive_table * /* That didn't merge into the previous dive. * Try to merge into next dive. */ - if (j < dives_to->nr && j > last_merged_into && - dive_endtime(dive_to_add) > dives_to->dives[j]->when) { - if (try_to_merge_into(dive_to_add, j, dives_to, prefer_imported, + if (j < dives_to.size() && (last_merged_into == std::string::npos || j > last_merged_into) && + dive_endtime(dive_to_add) > dives_to[j]->when) { + if (try_to_merge_into(dive_to_add, dives_to[j], prefer_imported, dives_to_add, dives_to_remove)) { delete dive_to_add; last_merged_into = j; @@ -915,9 +910,6 @@ static bool merge_dive_tables(struct dive_table *dives_from, struct dive_table * dive_to_add->divetrip = trip; } - /* we took care of all dives, clean up the import table */ - dives_from->nr = 0; - return sequence_changed; } @@ -1006,10 +998,12 @@ static bool try_to_merge_trip(dive_trip &trip_import, struct dive_table *import_ { for (auto &trip_old: *divelog.trips) { if (trips_overlap(trip_import, *trip_old)) { - *sequence_changed |= merge_dive_tables(&trip_import.dives, import_table, &trip_old->dives, + *sequence_changed |= merge_dive_tables(trip_import.dives, import_table, trip_old->dives, prefer_imported, trip_old.get(), dives_to_add, dives_to_remove, start_renumbering_at); + /* we took care of all dives of the trip, clean up the table */ + trip_import.dives.clear(); return true; } } @@ -1017,6 +1011,17 @@ static bool try_to_merge_trip(dive_trip &trip_import, struct dive_table *import_ return false; } +// Helper function to convert a table of owned dives into a table of non-owning pointers. +// Used to merge *all* dives of a log into a different table. +static std::vector dive_table_to_non_owning(const dive_table &dives) +{ + std::vector res; + res.reserve(dives.nr); + for (int i = 0; i < dives.nr; ++i) + res.push_back(dives.dives[i]); + return res; +} + /* Process imported dives: take a table of dives to be imported and * generate five lists: * 1) Dives to be added @@ -1139,9 +1144,7 @@ void process_imported_dives(struct divelog *import_log, int flags, /* If no trip to merge-into was found, add trip as-is. * First, add dives to list of dives to add */ - for (j = 0; j < trip_import->dives.nr; j++) { - struct dive *d = trip_import->dives.dives[j]; - + for (struct dive *d: trip_import->dives) { /* Add dive to list of dives to-be-added. */ insert_dive(dives_to_add, d); sequence_changed |= !dive_is_after_last(d); @@ -1149,7 +1152,7 @@ void process_imported_dives(struct divelog *import_log, int flags, remove_dive(d, import_log->dives.get()); } - trip_import->dives.nr = 0; /* Caller is responsible for adding dives to trip */ + trip_import->dives.clear(); /* Caller is responsible for adding dives to trip */ /* Finally, add trip to list of trips to add */ trips_to_add.put(std::move(trip_import)); @@ -1175,7 +1178,10 @@ void process_imported_dives(struct divelog *import_log, int flags, /* The remaining dives in import_log->dives are those that don't belong to * a trip and the caller does not want them to be associated to a * new trip. Merge them into the global table. */ - sequence_changed |= merge_dive_tables(import_log->dives.get(), NULL, divelog.dives.get(), flags & IMPORT_PREFER_IMPORTED, NULL, + sequence_changed |= merge_dive_tables(dive_table_to_non_owning(*import_log->dives), + import_log->dives.get(), + dive_table_to_non_owning(*divelog.dives), + flags & IMPORT_PREFER_IMPORTED, NULL, dives_to_add, dives_to_remove, &start_renumbering_at); } @@ -1265,9 +1271,9 @@ static int comp_dive_to_trip(struct dive *a, struct dive_trip *b) { /* This should never happen, nevertheless don't crash on trips * with no (or worse a negative number of) dives. */ - if (!b || b->dives.nr <= 0) + if (!b || b->dives.empty()) return -1; - return comp_dives(a, b->dives.dives[0]); + return comp_dives(a, b->dives[0]); } static int comp_dive_or_trip(struct dive_or_trip a, struct dive_or_trip b) diff --git a/core/divelog.cpp b/core/divelog.cpp index 837746db3..208e583eb 100644 --- a/core/divelog.cpp +++ b/core/divelog.cpp @@ -43,7 +43,7 @@ void divelog::delete_single_dive(int idx) if (trip) trips->sort(); - if (trip && trip->dives.nr == 0) + if (trip && trip->dives.empty()) trips->pull(trip); unregister_dive_from_dive_site(dive); delete_dive_from_table(dives.get(), idx); diff --git a/core/range.h b/core/range.h index 76c3ac3e7..7f5bc0f89 100644 --- a/core/range.h +++ b/core/range.h @@ -176,7 +176,24 @@ int index_of_if(const Range &range, Func f) template bool range_contains(const Range &v, const Element &item) { - return std::find(v.begin(), v.end(), item) != v.end(); + return std::find(std::begin(v), std::end(v), item) != v.end(); +} + +// Insert into an already sorted range +template +void range_insert_sorted(Range &v, Element &item, Comp &comp) +{ + auto it = std::lower_bound(std::begin(v), std::end(v), item, + [&comp](auto &a, auto &b) { return comp(a, b) < 0; }); + v.insert(it, std::move(item)); +} + +template +void range_remove(Range &v, const Element &item) +{ + auto it = std::find(std::begin(v), std::end(v), item); + if (it != std::end(v)) + v.erase(it); } #endif diff --git a/core/save-html.cpp b/core/save-html.cpp index 9b164fb7f..494d1dd0a 100644 --- a/core/save-html.cpp +++ b/core/save-html.cpp @@ -407,12 +407,10 @@ static void write_no_trip(struct membuffer *b, int *dive_no, bool selected_only, static void write_trip(struct membuffer *b, dive_trip *trip, int *dive_no, bool selected_only, const char *photos_dir, const bool list_only, char *sep) { - const struct dive *dive; const char *separator = ""; bool found_sel_dive = 0; - for (int i = 0; i < trip->dives.nr; i++) { - dive = trip->dives.dives[i]; + for (auto dive: trip->dives) { if (!dive->selected && selected_only) continue; diff --git a/core/selection.cpp b/core/selection.cpp index 69b396488..c201bf8ec 100644 --- a/core/selection.cpp +++ b/core/selection.cpp @@ -224,7 +224,7 @@ void setTripSelection(dive_trip *trip, dive *currentDive) for (auto &t: *divelog.trips) t->selected = t.get() == trip; - amount_selected = trip->dives.nr; + amount_selected = static_cast(trip->dives.size()); amount_trips_selected = 1; emit diveListNotifier.tripSelected(trip, currentDive); diff --git a/core/string-format.cpp b/core/string-format.cpp index 1f6cf48e0..5aa22f66c 100644 --- a/core/string-format.cpp +++ b/core/string-format.cpp @@ -315,7 +315,7 @@ QString formatTripTitle(const dive_trip &trip) QString formatTripTitleWithDives(const dive_trip &trip) { - int nr = trip.dives.nr; + int nr = static_cast(trip.dives.size()); return formatTripTitle(trip) + " " + gettextFromC::tr("(%n dive(s))", "", nr); } diff --git a/core/trip.cpp b/core/trip.cpp index ac748dd8e..efac1f219 100644 --- a/core/trip.cpp +++ b/core/trip.cpp @@ -3,11 +3,11 @@ #include "trip.h" #include "dive.h" #include "divelog.h" +#include "errorhelper.h" +#include "range.h" #include "subsurface-time.h" #include "subsurface-string.h" #include "selection.h" -#include "table.h" -#include "core/errorhelper.h" #ifdef DEBUG_TRIP void dump_trip_list() @@ -23,7 +23,7 @@ void dump_trip_list() trip->autogen ? "autogen " : "", i + 1, trip->location.c_str(), tm.tm_year, tm.tm_mon + 1, tm.tm_mday, tm.tm_hour, tm.tm_min, tm.tm_sec, - trip->dives.nr, trip.get()); + static_cast(trip->dives.size()), trip.get()); last_time = trip_date(*trip); } printf("-----\n"); @@ -34,23 +34,20 @@ dive_trip::dive_trip() : id(dive_getUniqID()) { } -dive_trip::~dive_trip() -{ - free(dives.dives); -} +dive_trip::~dive_trip() = default; timestamp_t trip_date(const struct dive_trip &trip) { - if (trip.dives.nr == 0) + if (trip.dives.empty()) return 0; - return trip.dives.dives[0]->when; + return trip.dives[0]->when; } static timestamp_t trip_enddate(const struct dive_trip &trip) { - if (trip.dives.nr == 0) + if (trip.dives.empty()) return 0; - return dive_endtime(trip.dives.dives[trip.dives.nr - 1]); + return dive_endtime(trip.dives.back()); } /* check if we have a trip right before / after this dive */ @@ -77,13 +74,13 @@ void add_dive_to_trip(struct dive *dive, dive_trip *trip) return; if (dive->divetrip) report_info("Warning: adding dive to trip that has trip set\n"); - insert_dive(&trip->dives, dive); + range_insert_sorted(trip->dives, dive, comp_dives); dive->divetrip = trip; } /* remove a dive from the trip it's associated to, but don't delete the * trip if this was the last dive in the trip. the caller is responsible - * for removing the trip, if the trip->dives.nr went to 0. + * for removing the trip, if the trip->dives.size() went to 0. */ struct dive_trip *unregister_dive_from_trip(struct dive *dive) { @@ -92,7 +89,7 @@ struct dive_trip *unregister_dive_from_trip(struct dive *dive) if (!trip) return NULL; - remove_dive(dive, &trip->dives); + range_remove(trip->dives, dive); dive->divetrip = NULL; return trip; } @@ -148,7 +145,7 @@ dive_trip *trip_table::get_by_uniq_id(int tripId) const bool trips_overlap(const struct dive_trip &t1, const struct dive_trip &t2) { /* First, handle the empty-trip cases. */ - if (t1.dives.nr == 0 || t2.dives.nr == 0) + if (t1.dives.empty() || t2.dives.empty()) return 0; if (trip_date(t1) < trip_date(t2)) @@ -247,13 +244,13 @@ int comp_trips(const struct dive_trip &a, const struct dive_trip &b) // address if both are empty. if (&a == &b) return 0; // reflexivity. shouldn't happen. - if (a.dives.nr <= 0 && b.dives.nr <= 0) + if (a.dives.empty() && b.dives.empty()) return &a < &b ? -1 : 1; - if (a.dives.nr <= 0) + if (a.dives.empty()) return -1; - if (b.dives.nr <= 0) + if (b.dives.empty()) return 1; - return comp_dives(a.dives.dives[0], b.dives.dives[0]); + return comp_dives(a.dives[0], b.dives[0]); } static bool is_same_day(timestamp_t trip_when, timestamp_t dive_when) @@ -274,18 +271,19 @@ static bool is_same_day(timestamp_t trip_when, timestamp_t dive_when) bool trip_is_single_day(const struct dive_trip &trip) { - if (trip.dives.nr <= 1) + if (trip.dives.size() <= 1) return true; - return is_same_day(trip.dives.dives[0]->when, - trip.dives.dives[trip.dives.nr - 1]->when); + return is_same_day(trip.dives.front()->when, + trip.dives.back()->when); } int trip_shown_dives(const struct dive_trip *trip) { - int res = 0; - for (int i = 0; i < trip->dives.nr; ++i) { - if (!trip->dives.dives[i]->hidden_by_filter) - res++; - } - return res; + return std::count_if(trip->dives.begin(), trip->dives.end(), + [](const dive *d) { return !d->hidden_by_filter; }); +} + +void dive_trip::sort_dives() +{ + std::sort(dives.begin(), dives.end(), [] (dive *d1, dive *d2) { return comp_dives(d1, d2) < 0; }); } diff --git a/core/trip.h b/core/trip.h index 6a48cf328..defd20a7a 100644 --- a/core/trip.h +++ b/core/trip.h @@ -11,13 +11,15 @@ struct dive_trip { std::string location; std::string notes; - struct dive_table dives = {}; + std::vector dives; int id; /* unique ID for this trip: used to pass trips through QML. */ /* Used by the io-routines to mark trips that have already been written. */ bool saved = false; bool autogen = false; bool selected = false; + void sort_dives(); + dive_trip(); ~dive_trip(); }; diff --git a/desktop-widgets/divelistview.cpp b/desktop-widgets/divelistview.cpp index 0783d1a48..9790cbe96 100644 --- a/desktop-widgets/divelistview.cpp +++ b/desktop-widgets/divelistview.cpp @@ -496,8 +496,8 @@ void DiveListView::selectionChanged(const QItemSelection &selected, const QItemS removeFromSelection.push_back(dive); } else if (dive_trip *trip = model->data(index, DiveTripModelBase::TRIP_ROLE).value()) { deselect_trip(trip); - for (int i = 0; i < trip->dives.nr; ++i) - removeFromSelection.push_back(trip->dives.dives[i]); + for (auto dive: trip->dives) + removeFromSelection.push_back(dive); } } for (const QModelIndex &index: newSelected.indexes()) { @@ -510,8 +510,8 @@ void DiveListView::selectionChanged(const QItemSelection &selected, const QItemS addToSelection.push_back(dive); } else if (dive_trip *trip = model->data(index, DiveTripModelBase::TRIP_ROLE).value()) { select_trip(trip); - for (int i = 0; i < trip->dives.nr; ++i) - addToSelection.push_back(trip->dives.dives[i]); + for (struct dive *d: trip->dives) + addToSelection.push_back(d); selectTripItems(index); } } diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp index d00410bb8..baaf16360 100644 --- a/qt-models/divetripmodel.cpp +++ b/qt-models/divetripmodel.cpp @@ -78,9 +78,9 @@ QString DiveTripModelBase::tripTitle(const dive_trip *trip) { if (!trip) return QString(); - QString numDives = tr("(%n dive(s))", "", trip->dives.nr); + QString numDives = tr("(%n dive(s))", "", static_cast(trip->dives.size())); int shown = trip_shown_dives(trip); - QString shownDives = shown != trip->dives.nr ? QStringLiteral(" ") + tr("(%L1 shown)").arg(shown) : QString(); + QString shownDives = shown != !trip->dives.empty() ? QStringLiteral(" ") + tr("(%L1 shown)").arg(shown) : QString(); QString title = QString::fromStdString(trip->location); if (title.isEmpty()) { @@ -88,7 +88,7 @@ QString DiveTripModelBase::tripTitle(const dive_trip *trip) QDateTime firstTime = timestampToDateTime(trip_date(*trip)); QString firstMonth = firstTime.toString("MMM"); QString firstYear = firstTime.toString("yyyy"); - QDateTime lastTime = timestampToDateTime(trip->dives.dives[0]->when); + QDateTime lastTime = timestampToDateTime(trip->dives[0]->when); QString lastMonth = lastTime.toString("MMM"); QString lastYear = lastTime.toString("yyyy"); if (lastMonth == firstMonth && lastYear == firstYear) @@ -107,7 +107,7 @@ QVariant DiveTripModelBase::tripData(const dive_trip *trip, int column, int role // Special roles for mobile switch(role) { case MobileListModel::TripIdRole: return QString::number(trip->id); - case MobileListModel::TripNrDivesRole: return trip->dives.nr; + case MobileListModel::TripNrDivesRole: return static_cast(trip->dives.size()); case MobileListModel::TripShortDateRole: return tripShortDate(trip); case MobileListModel::TripTitleRole: return tripTitle(trip); case MobileListModel::TripLocationRole: return QString::fromStdString(trip->location); @@ -126,7 +126,7 @@ QVariant DiveTripModelBase::tripData(const dive_trip *trip, int column, int role case DiveTripModelBase::NR: QString shownText; int countShown = trip_shown_dives(trip); - if (countShown < trip->dives.nr) + if (countShown < static_cast(trip->dives.size())) shownText = tr("(%1 shown)").arg(countShown); return formatTripTitleWithDives(*trip) + " " + shownText; } @@ -1696,9 +1696,9 @@ void DiveTripModelList::tripSelected(dive_trip *trip, dive *currentDive) // In the list view, there are no trips, so simply transform this into // a dive selection. QVector dives; - dives.reserve(trip->dives.nr); - for (int i = 0; i < trip->dives.nr; ++i) - dives.push_back(trip->dives.dives[i]); + dives.reserve(trip->dives.size()); + for (auto dive: trip->dives) + dives.push_back(dive); divesSelectedSlot(dives, currentDive, -1); }