Filter: separate backend from frontend logic

The filter code was an unholy intermixture of backend and frontend
logic, which made it hard to access it from outside of the UI.
Notably, it expected that Qt would call filterAcceptsRow on all rows.
For trip-view, apparently the filter functions were called twice
(once for filtering the trip, then for filtering the individual dives).

Make the filtering explicit, by calling showDive() for all dives in
MultiFilterSortModel::myInvalidate(), setting the hidden_by_filter
flags accordingly and ultimately invalidating the filter.

The UI code only accesses the hidden_by_filter flag set previously.

The "justCleared" flag can then be removed, since accessing the filter
does not have side effects. Moreover, there is no noticeable performance
gain by returning out early.

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

2 files changed, 22 insertions, 42 deletions

diff --git a/qt-models/filtermodels.cpp b/qt-models/filtermodels.cpp
index 91c524c68..1943056f7 100644
--- a/qt-models/filtermodels.cpp
+++ b/qt-models/filtermodels.cpp
@@ -362,7 +362,6 @@ void LocationFilterModel::addName(const QString &newName)
 
 MultiFilterSortModel::MultiFilterSortModel(QObject *parent) : QSortFilterProxyModel(parent),
 	divesDisplayed(0),
-	justCleared(false),
 	curr_dive_site(NULL)
 {
 }
@@ -376,7 +375,7 @@ bool MultiFilterSortModel::showDive(const struct dive *d) const
 		return same_string(ds->name, curr_dive_site->name) || ds->uuid == curr_dive_site->uuid;
 	}
 
-	if (justCleared || models.isEmpty())
+	if (models.isEmpty())
 		return true;
 
 	for (const FilterModelBase *model: models) {
@@ -393,42 +392,23 @@ bool MultiFilterSortModel::filterAcceptsRow(int source_row, const QModelIndex &s
 	QVariant diveVariant = sourceModel()->data(index0, DiveTripModel::DIVE_ROLE);
 	struct dive *d = (struct dive *)diveVariant.value<void *>();
 
-	if (d) {
-		// Current row is a dive
-		bool show = showDive(d);
-		filter_dive(d, show);
-		return show;
-	}
+	// For dives, simply check the hidden_by_filter flag
+	if (d)
+		return !d->hidden_by_filter;
 
-	// From here on, the current row is a trip
-	if (curr_dive_site) {
-		bool showTrip = false;
-		for (int i = 0; i < sourceModel()->rowCount(index0); i++) {
-			QModelIndex child = sourceModel()->index(i, 0, index0);
-			d = (struct dive *)sourceModel()->data(child, DiveTripModel::DIVE_ROLE).value<void *>();
-			dive_site *ds = get_dive_site_by_uuid(d->dive_site_uuid);
-			if (!ds)
-				continue;
-			if (same_string(ds->name, curr_dive_site->name) || ds->uuid == curr_dive_site->uuid) {
-				if (ds->uuid != curr_dive_site->uuid) {
-					qWarning() << "Warning, two different dive sites with same name have a different id"
-						   << ds->uuid << "and" << curr_dive_site->uuid;
-				}
-				showTrip = true; // do not shortcircuit the loop or the counts will be wrong
-			}
-		}
-		return showTrip;
-	}
+	// Since this is not a dive, it must be a trip
+	QVariant tripVariant = sourceModel()->data(index0, DiveTripModel::TRIP_ROLE);
+	dive_trip *trip = (dive_trip *)tripVariant.value<void *>();
 
-	if (justCleared || models.isEmpty())
-		return true;
+	if (!trip)
+		return false; // Oops. Neither dive nor trip, something is seriously wrong.
 
-	bool showTrip = false;
-	for (int i = 0; i < sourceModel()->rowCount(index0); i++) {
-		if (filterAcceptsRow(i, index0))
-			showTrip = true; // do not shortcircuit the loop or the counts will be wrong
+	// Show the trip if any dive is visible
+	for (d = trip->dives; d; d = d->next) {
+		if (!d->hidden_by_filter)
+			return true;
 	}
-	return showTrip;
+	return false;
 }
 
 void MultiFilterSortModel::myInvalidate()
@@ -440,6 +420,14 @@ void MultiFilterSortModel::myInvalidate()
 
 	divesDisplayed = 0;
 
+	// Apply filter for each dive
+	for_each_dive (i, d) {
+		bool show = showDive(d);
+		filter_dive(d, show);
+		if (show)
+			divesDisplayed++;
+	}
+
 	invalidateFilter();
 
 	// first make sure the trips are no longer shown as selected
@@ -464,11 +452,6 @@ void MultiFilterSortModel::myInvalidate()
 		dlv->selectDives(curSelectedDives);
 	}
 
-	for_each_dive (i, d) {
-		if (!d->hidden_by_filter)
-			divesDisplayed++;
-	}
-
 	emit filterFinished();
 
 	if (curr_dive_site) {
@@ -491,11 +474,9 @@ void MultiFilterSortModel::removeFilterModel(FilterModelBase *model)
 
 void MultiFilterSortModel::clearFilter()
 {
-	justCleared = true;
 	Q_FOREACH (FilterModelBase *iface, models) {
 		iface->clearFilter();
 	}
-	justCleared = false;
 	myInvalidate();
 }
 
diff --git a/qt-models/filtermodels.h b/qt-models/filtermodels.h
index 98212979b..bcbbf000a 100644
--- a/qt-models/filtermodels.h
+++ b/qt-models/filtermodels.h
@@ -111,7 +111,6 @@ signals:
 private:
 	MultiFilterSortModel(QObject *parent = 0);
 	QList<FilterModelBase *> models;
-	bool justCleared;
 	struct dive_site *curr_dive_site;
 };