Dive list: move sort-functionality into core

To make sorting more controlled, move all sorting functions into
the core. For this, introduce a "dive_or_trip" structure, which
represents a top-level item. Adapt the DiveTripModel accordingly.
There are now three sorting functions:
	1) dive_less_than
	2) trip_less_than
	3) dive_or_trip_less_than
These should be used by all sorting code. By moving them to a
single place, the mess can hopefully be cleaned up.

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

4 files changed, 72 insertions, 57 deletions

diff --git a/core/dive.h b/core/dive.h
index 7356f91a1..76f01a8c0 100644
--- a/core/dive.h
+++ b/core/dive.h
@@ -338,6 +338,12 @@ struct dive {
 	unsigned char git_id[20];
 };
 
+/* For the top-level list: an entry is either a dive or a trip */
+struct dive_or_trip {
+	struct dive *dive;
+	struct dive_trip *trip;
+};
+
 extern void invalidate_dive_cache(struct dive *dive);
 extern bool dive_cache_is_valid(const struct dive *dive);
 
@@ -557,6 +563,7 @@ extern int legacy_format_o2pressures(const struct dive *dive, const struct divec
 
 extern bool dive_less_than(const struct dive *a, const struct dive *b);
 extern bool trip_less_than(const struct dive_trip *a, const struct dive_trip *b);
+extern bool dive_or_trip_less_than(struct dive_or_trip a, struct dive_or_trip b);
 extern void sort_table(struct dive_table *table);
 extern struct dive *fixup_dive(struct dive *dive);
 extern void fixup_dc_duration(struct divecomputer *dc);
diff --git a/core/divelist.c b/core/divelist.c
index f9010b958..0fea7594e 100644
--- a/core/divelist.c
+++ b/core/divelist.c
@@ -40,6 +40,7 @@
  * void remove_autogen_trips()
  * bool dive_less_than(const struct dive *a, const struct dive *b)
  * bool trip_less_than(const struct dive_trip *a, const struct dive_trip *b)
+ * bool dive_or_trip_less_than(struct dive_or_trip a, struct dive_or_trip b)
  * void sort_table(struct dive_table *table)
  * bool is_trip_before_after(const struct dive *dive, bool before)
  * void delete_dive_from_table(struct dive_table *table, int idx)
@@ -1559,7 +1560,7 @@ void process_imported_dives(struct dive_table *import_table, bool prefer_importe
 		struct dive *dive_to_add = import_table->dives[i];
 
 		/* Find insertion point. */
-		while (j < dive_table.nr && dive_table.dives[j]->when < dive_to_add->when)
+		while (j < dive_table.nr && dive_less_than(dive_table.dives[j], dive_to_add))
 			j++;
 
 		/* Try to merge into previous dive. */
@@ -1742,13 +1743,11 @@ bool dive_less_than(const struct dive *a, const struct dive *b)
 	return comp_dives(a, b) < 0;
 }
 
-/* Trips are compared according to the first dive in the trip.
- * Even though it shouldn't happen, take care about "empty" trips.
- * Since a dive can only belong to one trip, no two trips should
- * compare as equal
- */
+/* Trips are compared according to the first dive in the trip. */
 static int comp_trips(const struct dive_trip *a, const struct dive_trip *b)
 {
+	/* This should never happen, nevertheless don't crash on trips
+	 * with no (or worse a negative number of) dives. */
 	if (a->dives.nr <= 0)
 		return b->dives.nr <= 0 ? 0 : -1;
 	if (b->dives.nr <= 0)
@@ -1761,6 +1760,33 @@ bool trip_less_than(const struct dive_trip *a, const struct dive_trip *b)
 	return comp_trips(a, b) < 0;
 }
 
+/* When comparing a dive to a trip, use the first dive of the trip. */
+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->dives.nr <= 0)
+		return -1;
+	return comp_dives(a, b->dives.dives[0]);
+}
+
+static int comp_dive_or_trip(struct dive_or_trip a, struct dive_or_trip b)
+{
+	if (a.dive && b.dive)
+		return comp_dives(a.dive, b.dive);
+	if (a.trip && b.trip)
+		return comp_trips(a.trip, b.trip);
+	if (a.dive)
+		return comp_dive_to_trip(a.dive, b.trip);
+	else
+		return -comp_dive_to_trip(b.dive, a.trip);
+}
+
+bool dive_or_trip_less_than(struct dive_or_trip a, struct dive_or_trip b)
+{
+	return comp_dive_or_trip(a, b) < 0;
+}
+
 static int sortfn(const void *_a, const void *_b)
 {
 	const struct dive *a = (const struct dive *)*(const void **)_a;
diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp
index 6eadbbb24..b90c13aed 100644
--- a/qt-models/divetripmodel.cpp
+++ b/qt-models/divetripmodel.cpp
@@ -300,7 +300,7 @@ int DiveTripModel::rowCount(const QModelIndex &parent) const
 
 	// Only trips have items
 	const Item &entry =  items[parent.row()];
-	return entry.trip ? entry.dives.size() : 0;
+	return entry.d_or_t.trip ? entry.dives.size() : 0;
 }
 
 static const quintptr noParent = ~(quintptr)0; // This is the "internalId" marker for top-level item
@@ -437,34 +437,33 @@ QVariant DiveTripModel::headerData(int section, Qt::Orientation orientation, int
 	return QVariant();
 }
 
-DiveTripModel::Item::Item(dive_trip *t, const QVector<dive *> &divesIn) : trip(t),
+DiveTripModel::Item::Item(dive_trip *t, const QVector<dive *> &divesIn) : d_or_t{nullptr, t},
 	dives(divesIn.toStdVector())
 {
 }
 
-DiveTripModel::Item::Item(dive_trip *t, dive *d) : trip(t),
+DiveTripModel::Item::Item(dive_trip *t, dive *d) : d_or_t{nullptr, t},
 	dives({ d })
 {
 }
 
-DiveTripModel::Item::Item(dive *d) : trip(nullptr),
-	dives({ d })
+DiveTripModel::Item::Item(dive *d) : d_or_t{d, nullptr}
 {
 }
 
 bool DiveTripModel::Item::isDive(const dive *d) const
 {
-	return !trip && dives.size() == 1 && dives[0] == d;
+	return d_or_t.dive == d;
 }
 
 dive *DiveTripModel::Item::getDive() const
 {
-	return !trip && dives.size() == 1 ? dives[0] : nullptr;
+	return d_or_t.dive;
 }
 
 timestamp_t DiveTripModel::Item::when() const
 {
-	return trip ? trip->when : dives[0]->when;
+	return d_or_t.trip ? d_or_t.trip->when : d_or_t.dive->when;
 }
 
 // Find a range of matching elements in a vector.
@@ -572,9 +571,9 @@ void DiveTripModel::setupModelData()
 		}
 
 		// Check if that trip is already known to us: search for the first item
-		// where item->trip is equal to trip.
+		// that corresponds to that trip
 		auto it = std::find_if(items.begin(), items.end(), [trip](const Item &item)
-				       { return item.trip == trip; });
+				       { return item.d_or_t.trip == trip; });
 		if (it == items.end()) {
 			// We didn't find an entry for this trip -> add one
 			items.emplace_back(trip, d);
@@ -593,28 +592,23 @@ void DiveTripModel::setLayout(DiveTripModel::Layout layout)
 	setupModelData();
 }
 
-QPair<dive_trip *, dive *> DiveTripModel::tripOrDive(const QModelIndex &index) const
+dive_or_trip DiveTripModel::tripOrDive(const QModelIndex &index) const
 {
 	if (!index.isValid())
 		return { nullptr, nullptr };
 
 	QModelIndex parent = index.parent();
 	// An invalid parent means that we're at the top-level
-	if (!parent.isValid()) {
-		const Item &entry = items[index.row()];
-		if (entry.trip)
-			return { entry.trip, nullptr };		// A trip
-		else
-			return { nullptr, entry.dives[0] };	// A dive
-	}
+	if (!parent.isValid())
+		return items[index.row()].d_or_t;
 
 	// Otherwise, we're at a leaf -> thats a dive
-	return { nullptr, items[parent.row()].dives[index.row()] };
+	return { items[parent.row()].dives[index.row()], nullptr };
 }
 
 dive *DiveTripModel::diveOrNull(const QModelIndex &index) const
 {
-	return tripOrDive(index).second;
+	return tripOrDive(index).dive;
 }
 
 QVariant DiveTripModel::data(const QModelIndex &index, int role) const
@@ -623,11 +617,11 @@ QVariant DiveTripModel::data(const QModelIndex &index, int role) const
 	if (role == Qt::FontRole)
 		return defaultModelFont();
 
-	auto entry = tripOrDive(index);
-	if (entry.first)
-		return tripData(entry.first, index.column(), role);
-	else if (entry.second)
-		return diveData(entry.second, index.column(), role);
+	dive_or_trip entry = tripOrDive(index);
+	if (entry.trip)
+		return tripData(entry.trip, index.column(), role);
+	else if (entry.dive)
+		return diveData(entry.dive, index.column(), role);
 	else
 		return QVariant();
 }
@@ -662,7 +656,7 @@ bool DiveTripModel::setData(const QModelIndex &index, const QVariant &value, int
 int DiveTripModel::findTripIdx(const dive_trip *trip) const
 {
 	for (int i = 0; i < (int)items.size(); ++i)
-		if (items[i].trip == trip)
+		if (items[i].d_or_t.trip == trip)
 			return i;
 	return -1;
 }
@@ -753,11 +747,8 @@ void DiveTripModel::addDivesToTrip(int trip, const QVector<dive *> &dives)
 // before the trip in the case of equal timestamps.
 bool DiveTripModel::dive_before_entry(const dive *d, const Item &entry)
 {
-	// Dives at the same time come afer trips in ascending mode, therefore use the "<" operator.
-	// This makes them appear as before trips in descending mode.
-	if (entry.trip)
-		return d->when < entry.trip->when;
-	return dive_less_than(d, entry.getDive());
+	dive_or_trip d_or_t { (dive *)d, nullptr };
+	return dive_or_trip_less_than(d_or_t, entry.d_or_t);
 }
 
 void DiveTripModel::divesAdded(dive_trip *trip, bool addTrip, const QVector<dive *> &dives)
@@ -1073,8 +1064,8 @@ bool DiveTripModel::lessThan(const QModelIndex &i1, const QModelIndex &i2) const
 	int row2 = i2.row();
 	if (row1 < 0 || row1 >= (int)items.size() || row2 < 0 || row2 >= (int)items.size())
 		return false;
-	const dive *d1 = items[i1.row()].dives[0];
-	const dive *d2 = items[i2.row()].dives[0];
+	const dive *d1 = items[i1.row()].d_or_t.dive;
+	const dive *d2 = items[i2.row()].d_or_t.dive;
 	// This is used as a second sort criterion: For equal values, sorting is chronologically *descending*.
 	int row_diff = row2 - row1;
 	switch (i1.column()) {
diff --git a/qt-models/divetripmodel.h b/qt-models/divetripmodel.h
index 4993c855f..8ff4524b3 100644
--- a/qt-models/divetripmodel.h
+++ b/qt-models/divetripmodel.h
@@ -81,24 +81,15 @@ private slots:
 private:
 	// The model has up to two levels. At the top level, we have either trips or dives
 	// that do not belong to trips. Such a top-level item is represented by the "Item"
-	// struct. Two cases two consider:
-	// 1) If "trip" is non-null, then this is a dive-trip and the dives are collected
-	// in the dives vector.  Note that in principle we could also get the dives in a
-	// trip from the backend, but there they are collected in a linked-list, which is
-	// quite inconvenient to access.
-	// 2) If "trip" is null, this is a dive and dives is supposed to contain exactly
-	// one element, which is the corresponding dive.
-	//
-	// Top-level items are ordered by timestamp. For dives, the core function
-	// dive_less_than is used, which guarantees a stable ordering even in the
-	// case of equal timestamps. For dives and trips, place dives before trips
-	// in the case of an equal timestamp. For trips with equal timestamps, the
-	// order is currently undefined. This is currently not a problem, because
-	// the core doesn't have a list of sorted trips. But nevertheless something
-	// to keep in mind.
+	// struct, which is based on the dive_or_trip structure.
+	// If it is a trip, additionally, the dives are collected in a vector.
+	// The items are ordered chronologically according to the dive_or_trip_less_than()
+	// function, which guarantees a stable ordering even in the case of equal timestamps.
+	// For dives and trips, it place dives chronologically after trips, so that in
+	// the default-descending view they are shown before trips.
 	struct Item {
-		dive_trip		*trip;
-		std::vector<dive *>	 dives;			// std::vector<> instead of QVector for insert() with three iterators
+		dive_or_trip		d_or_t;
+		std::vector<dive *>	dives;			// std::vector<> instead of QVector for insert() with three iterators
 		Item(dive_trip *t, const QVector<dive *> &dives);
 		Item(dive_trip *t, dive *d);			// Initialize a trip with one dive
 		Item(dive *d);					// Initialize a top-level dive
@@ -126,7 +117,7 @@ private:
 	void addDivesToTrip(int idx, const QVector<dive *> &dives);
 
 	dive *diveOrNull(const QModelIndex &index) const;	// Returns a dive if this index represents a dive, null otherwise
-	QPair<dive_trip *, dive *> tripOrDive(const QModelIndex &index) const;
+	dive_or_trip tripOrDive(const QModelIndex &index) const;
 								// Returns either a pointer to a trip or a dive, or twice null of index is invalid
 								// null, something is really wrong
 	void setupModelData();