From 0ef497c3c9014f976c1cc5a67b9d1c15b7188496 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Mon, 3 Jun 2024 21:50:08 +0200 Subject: [PATCH] core: make split_dive() and related functions return unique_ptrs This prepares for turning the dive table into a list of owning pointers. Signed-off-by: Berthold Stoeger --- commands/command_divelist.cpp | 36 ++------- commands/command_divelist.h | 2 +- core/dive.cpp | 135 ++++++++++++++-------------------- core/dive.h | 8 +- 4 files changed, 71 insertions(+), 110 deletions(-) diff --git a/commands/command_divelist.cpp b/commands/command_divelist.cpp index ec73a2a73..46d4f0a30 100644 --- a/commands/command_divelist.cpp +++ b/commands/command_divelist.cpp @@ -9,8 +9,6 @@ #include "qt-models/filtermodels.h" #include "core/divefilter.h" -#include - namespace Command { // Helper function that takes care to unselect trips that are removed from the backend @@ -801,10 +799,7 @@ MergeTrips::MergeTrips(dive_trip *trip1, dive_trip *trip2) divesToMove.tripsToAdd.push_back(std::move(newTrip)); } -// std::array is the same as struct *dive[2], with the fundamental -// difference that it can be returned from functions. Thus, this constructor -// can be chained with the result of a function. -SplitDivesBase::SplitDivesBase(dive *d, std::array newDives) +SplitDivesBase::SplitDivesBase(dive *d, std::array, 2> newDives) { // If either of the new dives is null, simply return. Empty arrays indicate that nothing is to be done. if (!newDives[0] || !newDives[1]) @@ -824,10 +819,10 @@ SplitDivesBase::SplitDivesBase(dive *d, std::array newDives) diveToSplit.dives.push_back(d); splitDives.dives.resize(2); - splitDives.dives[0].dive.reset(newDives[0]); + splitDives.dives[0].dive = std::move(newDives[0]); splitDives.dives[0].trip = d->divetrip; splitDives.dives[0].site = d->dive_site; - splitDives.dives[1].dive.reset(newDives[1]); + splitDives.dives[1].dive = std::move(newDives[1]); splitDives.dives[1].trip = d->divetrip; splitDives.dives[1].site = d->dive_site; } @@ -856,16 +851,13 @@ void SplitDivesBase::undoit() setSelection(diveToSplit.dives, diveToSplit.dives[0], -1); } -static std::array doSplitDives(const dive *d, duration_t time) +static std::array, 2> doSplitDives(const dive *d, duration_t time) { // Split the dive - dive *new1, *new2; if (time.seconds < 0) - split_dive(d, &new1, &new2); + return split_dive(*d); else - split_dive_at_time(d, time, &new1, &new2); - - return { new1, new2 }; + return split_dive_at_time(*d, time); } SplitDives::SplitDives(dive *d, duration_t time) : SplitDivesBase(d, doSplitDives(d, time)) @@ -873,20 +865,8 @@ SplitDives::SplitDives(dive *d, duration_t time) : SplitDivesBase(d, doSplitDive setText(Command::Base::tr("split dive")); } -static std::array splitDiveComputer(const dive *d, int dc_num) -{ - // Refuse to do anything if the dive has only one dive computer. - // Yes, this should have been checked by the UI, but let's just make sure. - if (d->dcs.size() <= 1) - return { nullptr, nullptr}; - - dive *new1, *new2; - split_divecomputer(d, dc_num, &new1, &new2); - - return { new1, new2 }; -} - -SplitDiveComputer::SplitDiveComputer(dive *d, int dc_num) : SplitDivesBase(d, splitDiveComputer(d, dc_num)) +SplitDiveComputer::SplitDiveComputer(dive *d, int dc_num) : + SplitDivesBase(d, split_divecomputer(*d, dc_num)) { setText(Command::Base::tr("split dive computer")); } diff --git a/commands/command_divelist.h b/commands/command_divelist.h index e7c232fe4..bdc0f28ec 100644 --- a/commands/command_divelist.h +++ b/commands/command_divelist.h @@ -196,7 +196,7 @@ struct MergeTrips : public TripBase { class SplitDivesBase : public DiveListBase { protected: - SplitDivesBase(dive *old, std::array newDives); + SplitDivesBase(dive *old, std::array, 2> newDives); private: void undoit() override; void redoit() override; diff --git a/core/dive.cpp b/core/dive.cpp index e02d3fe34..8c769e2c0 100644 --- a/core/dive.cpp +++ b/core/dive.cpp @@ -49,6 +49,7 @@ dive::dive() : dcs(1) id = dive_getUniqID(); } +dive::dive(const dive &) = default; dive::dive(dive &&) = default; dive &dive::operator=(const dive &) = default; dive::~dive() = default; @@ -180,13 +181,6 @@ void copy_dive(const struct dive *s, struct dive *d) invalidate_dive_cache(d); } -static void copy_dive_onedc(const struct dive *s, const struct divecomputer &sdc, struct dive *d) -{ - copy_dive(s, d); - d->dcs.clear(); - d->dcs.push_back(sdc); -} - /* make a clone of the source dive and clean out the source dive; * this allows us to create a dive on the stack and then * add it to the divelist. */ @@ -2340,19 +2334,6 @@ struct dive *merge_dives(const struct dive *a, const struct dive *b, int offset, return res; } -// copy_dive(), but retaining the new ID for the copied dive -static struct dive *create_new_copy(const struct dive *from) -{ - struct dive *to = new dive; - - // dive creation gave us a new ID, we just need to - // make sure it's not overwritten. - int id = to->id; - copy_dive(from, to); - to->id = id; - return to; -} - struct start_end_pressure { pressure_t start; pressure_t end; @@ -2412,44 +2393,43 @@ static void force_fixup_dive(struct dive *d) * Moreover, on failure both output dives are set to NULL. * On success, the newly allocated dives are returned in out1 and out2. */ -static int split_dive_at(const struct dive *dive, int a, int b, struct dive **out1, struct dive **out2) +static std::array, 2> split_dive_at(const struct dive &dive, int a, int b) { - int nr; - int32_t t; - struct dive *d1, *d2; - struct divecomputer *dc1, *dc2; + int nr = get_divenr(&dive); /* if we can't find the dive in the dive list, don't bother */ - if ((nr = get_divenr(dive)) < 0) - return -1; + if (nr < 0) + return {}; /* Splitting should leave at least 3 samples per dive */ - if (a < 3 || static_cast(b + 4) > dive->dcs[0].samples.size()) - return -1; + if (a < 3 || static_cast(b + 4) > dive.dcs[0].samples.size()) + return {}; /* We're not trying to be efficient here.. */ - d1 = create_new_copy(dive); - d2 = create_new_copy(dive); - d1->divetrip = d2->divetrip = 0; + auto d1 = std::make_unique(dive); + auto d2 = std::make_unique(dive); + d1->id = dive_getUniqID(); + d2->id = dive_getUniqID(); + d1->divetrip = d2->divetrip = nullptr; /* now unselect the first first segment so we don't keep all * dives selected by mistake. But do keep the second one selected * so the algorithm keeps splitting the dive further */ d1->selected = false; - dc1 = &d1->dcs[0]; - dc2 = &d2->dcs[0]; + struct divecomputer &dc1 = d1->dcs[0]; + struct divecomputer &dc2 = d2->dcs[0]; /* * Cut off the samples of d1 at the beginning * of the interval. */ - dc1->samples.resize(a); + dc1.samples.resize(a); /* And get rid of the 'b' first samples of d2 */ - dc2->samples.erase(dc2->samples.begin(), dc2->samples.begin() + b); + dc2.samples.erase(dc2.samples.begin(), dc2.samples.begin() + b); /* Now the secondary dive computers */ - t = dc2->samples[0].time.seconds; + int32_t t = dc2.samples[0].time.seconds; for (auto it1 = d1->dcs.begin() + 1; it1 != d1->dcs.end(); ++it1) { auto it = std::find_if(it1->samples.begin(), it1->samples.end(), [t](auto &sample) { return sample.time.seconds >= t; }); @@ -2491,8 +2471,8 @@ static int split_dive_at(const struct dive *dive, int a, int b, struct dive **ou ++it2; } - force_fixup_dive(d1); - force_fixup_dive(d2); + force_fixup_dive(d1.get()); + force_fixup_dive(d2.get()); /* * Was the dive numbered? If it was the last dive, then we'll @@ -2506,9 +2486,7 @@ static int split_dive_at(const struct dive *dive, int a, int b, struct dive **ou d2->number = 0; } - *out1 = d1; - *out2 = d2; - return nr; + return { std::move(d1), std::move(d2) }; } /* in freedive mode we split for as little as 10 seconds on the surface, @@ -2530,16 +2508,12 @@ static bool should_split(const struct divecomputer *dc, int t1, int t2) * * In other words, this is a (simplified) reversal of the dive merging. */ -int split_dive(const struct dive *dive, struct dive **new1, struct dive **new2) +std::array, 2> split_dive(const struct dive &dive) { - *new1 = *new2 = NULL; - if (!dive) - return -1; - - const struct divecomputer *dc = &dive->dcs[0]; + const struct divecomputer *dc = &dive.dcs[0]; bool at_surface = true; if (dc->samples.empty()) - return -1; + return {}; auto surface_start = dc->samples.begin(); for (auto it = dc->samples.begin() + 1; it != dc->samples.end(); ++it) { bool surface_sample = it->depth.mm < SURFACE_THRESHOLD; @@ -2565,24 +2539,21 @@ int split_dive(const struct dive *dive, struct dive **new1, struct dive **new2) if (!should_split(dc, surface_start->time.seconds, std::prev(it)->time.seconds)) continue; - return split_dive_at(dive, surface_start - dc->samples.begin(), it - dc->samples.begin() - 1, new1, new2); + return split_dive_at(dive, surface_start - dc->samples.begin(), it - dc->samples.begin() - 1); } - return -1; + return {}; } -int split_dive_at_time(const struct dive *dive, duration_t time, struct dive **new1, struct dive **new2) +std::array, 2> split_dive_at_time(const struct dive &dive, duration_t time) { - if (!dive) - return -1; - - auto it = std::find_if(dive->dcs[0].samples.begin(), dive->dcs[0].samples.end(), + auto it = std::find_if(dive.dcs[0].samples.begin(), dive.dcs[0].samples.end(), [time](auto &sample) { return sample.time.seconds >= time.seconds; }); - if (it == dive->dcs[0].samples.end()) - return -1; - size_t idx = it - dive->dcs[0].samples.begin(); + if (it == dive.dcs[0].samples.end()) + return {}; + size_t idx = it - dive.dcs[0].samples.begin(); if (idx < 1) - return -1; - return split_dive_at(dive, static_cast(idx), static_cast(idx - 1), new1, new2); + return {}; + return split_dive_at(dive, static_cast(idx), static_cast(idx - 1)); } /* @@ -2752,29 +2723,37 @@ struct dive *clone_delete_divecomputer(const struct dive *d, int dc_number) * The dives will not be associated with a trip. * On error, both output parameters are set to NULL. */ -void split_divecomputer(const struct dive *src, int num, struct dive **out1, struct dive **out2) +std::array, 2> split_divecomputer(const struct dive &src, int num) { - const struct divecomputer *srcdc = get_dive_dc(src, num); + if (num < 0 || src.dcs.size() < 2 || static_cast(num) >= src.dcs.size()) + return {}; - if (src && srcdc) { - // Copy the dive, but only using the selected dive computer - *out2 = new dive; - copy_dive_onedc(src, *srcdc, *out2); + // Copy the dive with full divecomputer list + auto out1 = std::make_unique(src); - // This will also make fixup_dive() to allocate a new dive id... - (*out2)->id = 0; - fixup_dive(*out2); + // Remove all DCs, stash them and copy the dive again. + // Then, we have to dives without DCs and a list of DCs. + std::vector dcs; + std::swap(out1->dcs, dcs); + auto out2 = std::make_unique(*out1); - // Copy the dive with all dive computers - *out1 = create_new_copy(src); + // Give the dives new unique ids and remove them from the trip. + out1->id = dive_getUniqID(); + out2->id = dive_getUniqID(); + out1->divetrip = out2->divetrip = NULL; - // .. and then delete the split-out dive computer - delete_divecomputer(*out1, num); - - (*out1)->divetrip = (*out2)->divetrip = NULL; - } else { - *out1 = *out2 = NULL; + // Now copy the divecomputers + out1->dcs.reserve(src.dcs.size() - 1); + for (auto [idx, dc]: enumerated_range(dcs)) { + auto &dcs = idx == num ? out2->dcs : out1->dcs; + dcs.push_back(std::move(dc)); } + + // Recalculate gas data, etc. + fixup_dive(out1.get()); + fixup_dive(out2.get()); + + return { std::move(out1), std::move(out2) }; } //Calculate O2 in best mix diff --git a/core/dive.h b/core/dive.h index 5255bee9e..caa516b48 100644 --- a/core/dive.h +++ b/core/dive.h @@ -10,6 +10,7 @@ #include "picture.h" // TODO: remove #include "tag.h" +#include #include #include #include @@ -74,6 +75,7 @@ struct dive { dive(); ~dive(); + dive(const dive &); dive(dive &&); dive &operator=(const dive &); }; @@ -143,7 +145,7 @@ extern void set_git_prefs(const char *prefs); extern struct dive *make_first_dc(const struct dive *d, int dc_number); extern struct dive *clone_delete_divecomputer(const struct dive *d, int dc_number); -void split_divecomputer(const struct dive *src, int num, struct dive **out1, struct dive **out2); +extern std::array, 2> split_divecomputer(const struct dive &src, int num); /* * Iterate over each dive, with the first parameter being the index @@ -192,8 +194,8 @@ extern pressure_t calculate_surface_pressure(const struct dive *dive); extern pressure_t un_fixup_surface_pressure(const struct dive *d); extern int get_dive_salinity(const struct dive *dive); extern int dive_getUniqID(); -extern int split_dive(const struct dive *dive, struct dive **new1, struct dive **new2); -extern int split_dive_at_time(const struct dive *dive, duration_t time, struct dive **new1, struct dive **new2); +extern std::array, 2> split_dive(const struct dive &dive); +extern std::array, 2> split_dive_at_time(const struct dive &dive, duration_t time); extern struct dive *merge_dives(const struct dive *a, const struct dive *b, int offset, bool prefer_downloaded, struct dive_trip **trip, struct dive_site **site); extern struct dive *try_to_merge(struct dive *a, struct dive *b, bool prefer_downloaded); extern void copy_events_until(const struct dive *sd, struct dive *dd, int dcNr, int time);