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<dive *>.

Add a helper functions to add/remove items in a sorted list.
These could be used elsewhere.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

commands/command_divelist.cpp

@@ -40,7 +40,7 @@ DiveToAdd DiveListBase::removeDive(struct dive *d, std::vector<std::unique_ptr<d
 	if (d->dive_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<dive_trip> 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<dive *> dives = stdToQt<dive *>(diveList);
@@ -794,10 +794,10 @@ MergeTrips::MergeTrips(dive_trip *trip1, dive_trip *trip2)
 	if (trip1 == trip2)
 		return;
 	std::unique_ptr<dive_trip> newTrip = combine_trips(trip1, trip2);
-	for (int i = 0; i < trip1->dives.nr; ++i)
+	for (dive *d: trip1->dives)
-		divesToMove.divesToMove.push_back( { trip1->dives.dives[i], newTrip.get() } );
+		divesToMove.divesToMove.push_back( { d, newTrip.get() } );
-	for (int i = 0; i < trip2->dives.nr; ++i)
+	for (dive *d: trip2->dives)
-		divesToMove.divesToMove.push_back( { trip2->dives.dives[i], newTrip.get() } );
+		divesToMove.divesToMove.push_back( { d, newTrip.get() } );
 	divesToMove.tripsToAdd.push_back(std::move(newTrip));
 }
 

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);
 	}
 

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,
+static bool merge_dive_tables(const std::vector<dive *> &dives_from, struct dive_table *delete_from,
-			      struct dive_table *dives_to,
+			      const std::vector<dive *> &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 */
+	size_t j = 0; /* Index in dives_to */
-	for (i = 0; i < dives_from->nr; i++) {
+	size_t last_merged_into = std::string::npos;
-		struct dive *dive_to_add = dives_from->dives[i];
+	for (auto dive_to_add: dives_from) {
-
-		if (delete_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 &&
+		if (j > 0 && (last_merged_into == std::string::npos || j > last_merged_into + 1) &&
-		    dive_endtime(dives_to->dives[j - 1]) > dive_to_add->when) {
+		    dive_endtime(dives_to[j - 1]) > dive_to_add->when) {
-			if (try_to_merge_into(dive_to_add, j - 1, dives_to, prefer_imported,
+			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 &&
+		if (j < dives_to.size() && (last_merged_into == std::string::npos || j > last_merged_into) &&
-		    dive_endtime(dive_to_add) > dives_to->dives[j]->when) {
+		    dive_endtime(dive_to_add) > dives_to[j]->when) {
-			if (try_to_merge_into(dive_to_add, j, dives_to, prefer_imported,
+			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 *> dive_table_to_non_owning(const dive_table &dives)
+{
+	std::vector<dive *> 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++) {
+		for (struct dive *d: trip_import->dives) {
-			struct dive *d = trip_import->dives.dives[j];
-
 			/* 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)

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);

core/range.h

@@ -176,7 +176,24 @@ int index_of_if(const Range &range, Func f)
 template<typename Range, typename Element>
 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<typename Range, typename Element, typename Comp>
+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<typename Range, typename Element>
+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

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++) {
+	for (auto dive: trip->dives) {
-		dive = trip->dives.dives[i];
 		if (!dive->selected && selected_only)
 			continue;
 

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<int>(trip->dives.size());
 	amount_trips_selected = 1;
 
 	emit diveListNotifier.tripSelected(trip, currentDive);

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<int>(trip.dives.size());
 	return formatTripTitle(trip) + " " +
 	       gettextFromC::tr("(%n dive(s))", "", nr);
 }

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<int>(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()
+dive_trip::~dive_trip() = default;
-{
-	free(dives.dives);
-}
 
 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,
+	return is_same_day(trip.dives.front()->when,
-			   trip.dives.dives[trip.dives.nr - 1]->when);
+			   trip.dives.back()->when);
 }
 
 int trip_shown_dives(const struct dive_trip *trip)
 {
-	int res = 0;
+	return std::count_if(trip->dives.begin(), trip->dives.end(),
-	for (int i = 0; i < trip->dives.nr; ++i) {
+			     [](const dive *d) { return !d->hidden_by_filter; });
-		if (!trip->dives.dives[i]->hidden_by_filter)
+}
-			res++;
+
-	}
+void dive_trip::sort_dives()
-	return res;
+{
+	std::sort(dives.begin(), dives.end(), [] (dive *d1, dive *d2) { return comp_dives(d1, d2) < 0; });
 }

core/trip.h

@@ -11,13 +11,15 @@ struct dive_trip
 {
 	std::string location;
 	std::string notes;
-	struct dive_table dives = {};
+	std::vector<dive *> 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();
 };

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<dive_trip *>()) {
 			deselect_trip(trip);
-			for (int i = 0; i < trip->dives.nr; ++i)
+			for (auto dive: trip->dives)
-				removeFromSelection.push_back(trip->dives.dives[i]);
+				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<dive_trip *>()) {
 			select_trip(trip);
-			for (int i = 0; i < trip->dives.nr; ++i)
+			for (struct dive *d: trip->dives)
-				addToSelection.push_back(trip->dives.dives[i]);
+				addToSelection.push_back(d);
 			selectTripItems(index);
 		}
 	}

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<int>(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<int>(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<int>(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<dive *> dives;
-	dives.reserve(trip->dives.nr);
+	dives.reserve(trip->dives.size());
-	for (int i = 0; i < trip->dives.nr; ++i)
+	for (auto dive: trip->dives)
-		dives.push_back(trip->dives.dives[i]);
+		dives.push_back(dive);
 
 	divesSelectedSlot(dives, currentDive, -1);
 }