map: use value semantics for MapLocation

This makes memory management more simple, as not explicit deletion
is necessary.

A rather large commit, because changing QVector<> to std::vector<>
is propagated up the call chain.

Adds a new range_contains() helper function for collection
types such as std::vector<>. I didn't want to call it
contains(), since we already have a contains function
for strings and let's keep argument overloading simple.

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

backend-shared/exportfuncs.cpp

@@ -318,9 +318,7 @@ std::vector<const dive_site *> getDiveSitesToExport(bool selectedOnly)
 	if (selectedOnly && DiveFilter::instance()->diveSiteMode()) {
 		// Special case in dive site mode: export all selected dive sites,
 		// not the dive sites of selected dives.
-		QVector<dive_site *> sites = DiveFilter::instance()->filteredDiveSites();
+		for (auto ds: DiveFilter::instance()->filteredDiveSites())
-		res.reserve(sites.size());
-		for (const dive_site *ds: sites)
 			res.push_back(ds);
 		return res;
 	}

core/divefilter.cpp

@@ -5,6 +5,7 @@
 #include "divelog.h"
 #include "gettextfromc.h"
 #include "qthelper.h"
+#include "range.h"
 #include "selection.h"
 #include "subsurface-qt/divelistnotifier.h"
 #if !defined(SUBSURFACE_MOBILE) && !defined(SUBSURFACE_DOWNLOADER)
@@ -61,7 +62,7 @@ ShownChange DiveFilter::update(const QVector<dive *> &dives) const
 	std::vector<dive *> removeFromSelection;
 	for (dive *d: dives) {
 		// There are three modes: divesite, fulltext, normal
-		bool newStatus = doDS        ? dive_sites.contains(d->dive_site) :
+		bool newStatus = doDS        ? range_contains(dive_sites, d->dive_site) :
 				 doFullText  ? fulltext_dive_matches(d, filterData.fullText, filterData.fulltextStringMode) && showDive(d) :
 					       showDive(d);
 		updateDiveStatus(d, newStatus, res, removeFromSelection);
@@ -91,7 +92,7 @@ ShownChange DiveFilter::updateAll() const
 	// There are three modes: divesite, fulltext, normal
 	if (diveSiteMode()) {
 		for_each_dive(i, d) {
-			bool newStatus = dive_sites.contains(d->dive_site);
+			bool newStatus = range_contains(dive_sites, d->dive_site);
 			updateDiveStatus(d, newStatus, res, removeFromSelection);
 		}
 	} else if (filterData.fullText.doit()) {
@@ -142,7 +143,7 @@ bool DiveFilter::showDive(const struct dive *d) const
 }
 
 #if !defined(SUBSURFACE_MOBILE) && !defined(SUBSURFACE_DOWNLOADER)
-void DiveFilter::startFilterDiveSites(QVector<dive_site *> ds)
+void DiveFilter::startFilterDiveSites(std::vector<dive_site *> ds)
 {
 	if (++diveSiteRefCount > 1) {
 		setFilterDiveSite(std::move(ds));
@@ -169,7 +170,7 @@ void DiveFilter::stopFilterDiveSites()
 #endif
 }
 
-void DiveFilter::setFilterDiveSite(QVector<dive_site *> ds)
+void DiveFilter::setFilterDiveSite(std::vector<dive_site *> ds)
 {
 	// If the filter didn't change, return early to avoid a full
 	// map reload. For a well-defined comparison, sort the vector first.
@@ -185,7 +186,7 @@ void DiveFilter::setFilterDiveSite(QVector<dive_site *> ds)
 	MainWindow::instance()->diveList->expandAll();
 }
 
-const QVector<dive_site *> &DiveFilter::filteredDiveSites() const
+const std::vector<dive_site *> &DiveFilter::filteredDiveSites() const
 {
 	return dive_sites;
 }

core/divefilter.h

@@ -45,9 +45,9 @@ public:
 	bool diveSiteMode() const; // returns true if we're filtering on dive site (on mobile always returns false)
 	std::vector<dive *> visibleDives() const;
 #ifndef SUBSURFACE_MOBILE
-	const QVector<dive_site *> &filteredDiveSites() const;
+	const std::vector<dive_site *> &filteredDiveSites() const;
-	void startFilterDiveSites(QVector<dive_site *> ds);
+	void startFilterDiveSites(std::vector<dive_site *> ds);
-	void setFilterDiveSite(QVector<dive_site *> ds);
+	void setFilterDiveSite(std::vector<dive_site *> ds);
 	void stopFilterDiveSites();
 #endif
 	void setFilter(const FilterData &data);
@@ -62,7 +62,7 @@ private:
 	void updateDiveStatus(dive *d, bool newStatus, ShownChange &change,
 			      std::vector<dive *> &removeFromSelection) const;
 
-	QVector<dive_site *> dive_sites;
+	std::vector<dive_site *> dive_sites;
 	FilterData filterData;
 	mutable int shown_dives;
 

core/range.h

@@ -98,4 +98,11 @@ int index_of_if(const Range &range, Func f)
 	return it == std::end(range) ? -1 : it - std::begin(range);
 }
 
+// Not really appropriate here, but oh my.
+template<typename Range, typename Element>
+bool range_contains(const Range &v, const Element &item)
+{
+	return std::find(v.begin(), v.end(), item) != v.end();
+}
+
 #endif

core/subsurface-string.h

@@ -2,6 +2,7 @@
 #ifndef SUBSURFACE_STRING_H
 #define SUBSURFACE_STRING_H
 
+#include <algorithm>
 #include <stdbool.h>
 #include <string.h>
 #include <time.h>

desktop-widgets/divelistview.cpp

@@ -22,6 +22,7 @@
 #include "commands/command_base.h"
 #include "core/errorhelper.h"
 #include "core/qthelper.h"
+#include "core/range.h"
 #include "core/trip.h"
 #include "desktop-widgets/divelistview.h"
 #include "core/metrics.h"
@@ -439,13 +440,13 @@ void DiveListView::selectDiveSitesOnMap(const std::vector<dive *> &dives)
 	// the dive-site selection is controlled by the filter not
 	// by the selected dives.
 	if (!DiveFilter::instance()->diveSiteMode()) {
-		QVector<dive_site *> selectedSites;
+		std::vector<dive_site *> selectedSites;
 		selectedSites.reserve(dives.size());
 		for (dive *d: dives) {
-			if (!d->hidden_by_filter && d->dive_site && !selectedSites.contains(d->dive_site))
+			if (!d->hidden_by_filter && d->dive_site && !range_contains(selectedSites, d->dive_site))
 				selectedSites.push_back(d->dive_site);
 		}
-		MapWidget::instance()->setSelected(selectedSites);
+		MapWidget::instance()->setSelected(std::move(selectedSites));
 	}
 #endif
 }

desktop-widgets/divelogexportdialog.cpp

@@ -207,7 +207,7 @@ void DiveLogExportDialog::on_buttonBox_accepted()
 				if (!filename.contains('.'))
 					filename.append(".xml");
 				QByteArray bt = QFile::encodeName(filename);
-				std::vector<const dive_site *> sites = getDiveSitesToExport(ui->exportSelected->isChecked());
+				auto sites = getDiveSitesToExport(ui->exportSelected->isChecked());
 				save_dive_sites_logic(bt.data(), sites.data(), (int)sites.size(), ui->anonymize->isChecked());
 			}
 		} else if (ui->exportImageDepths->isChecked()) {

desktop-widgets/divesitelistview.cpp

@@ -115,14 +115,14 @@ void DiveSiteListView::on_filterText_textChanged(const QString &text)
 	model->setFilter(text);
 }
 
-QVector<dive_site *> DiveSiteListView::selectedDiveSites()
+std::vector<dive_site *> DiveSiteListView::selectedDiveSites()
 {
 	const QModelIndexList indices = ui.diveSites->view()->selectionModel()->selectedRows();
-	QVector<dive_site *> sites;
+	std::vector<dive_site *> sites;
 	sites.reserve(indices.size());
 	for (const QModelIndex &idx: indices) {
 		struct dive_site *ds = model->getDiveSite(idx);
-		sites.append(ds);
+		sites.push_back(ds);
 	}
 	return sites;
 }

desktop-widgets/divesitelistview.h

@@ -22,7 +22,7 @@ private slots:
 private:
 	Ui::DiveSiteListView ui;
 	DiveSiteSortedModel *model;
-	QVector<dive_site *> selectedDiveSites();
+	std::vector<dive_site *> selectedDiveSites();
 	void hideEvent(QHideEvent *) override;
 	void showEvent(QShowEvent *) override;
 };

desktop-widgets/locationinformation.cpp

@@ -260,7 +260,7 @@ void LocationInformationWidget::initFields(dive_site *ds)
 		filter_model.set(ds, ds->location);
 		updateLabels();
 		enableLocationButtons(dive_site_has_gps_location(ds));
-		DiveFilter::instance()->startFilterDiveSites(QVector<dive_site *>{ ds });
+		DiveFilter::instance()->startFilterDiveSites(std::vector<dive_site *>{ ds });
 		filter_model.invalidate();
 	} else {
 		filter_model.set(0, location_t());

desktop-widgets/mapwidget.cpp

@@ -79,10 +79,10 @@ bool MapWidget::editMode() const
 	return isReady && m_mapHelper->editMode();
 }
 
-void MapWidget::setSelected(const QVector<dive_site *> &divesites)
+void MapWidget::setSelected(std::vector<dive_site *> divesites)
 {
 	CHECK_IS_READY_RETURN_VOID();
-	m_mapHelper->setSelected(divesites);
+	m_mapHelper->setSelected(std::move(divesites));
 	m_mapHelper->centerOnSelectedDiveSite();
 }
 
@@ -97,7 +97,7 @@ void MapWidget::selectedDivesChanged(const QList<int> &list)
 		if (dive *d = get_dive(idx))
 			selection.push_back(d);
 	}
-	setSelection(selection, current_dive, -1);
+	setSelection(std::move(selection), current_dive, -1);
 }
 
 void MapWidget::coordinatesChanged(struct dive_site *ds, const location_t &location)

desktop-widgets/mapwidget.h

@@ -4,6 +4,7 @@
 
 #include "core/units.h"
 #include "core/subsurface-qt/divelistnotifier.h"
+#include <vector>
 #include <QQuickWidget>
 #include <QList>
 
@@ -23,7 +24,7 @@ public:
 
 	static MapWidget *instance();
 	void reload();
-	void setSelected(const QVector<dive_site *> &divesites);
+	void setSelected(std::vector<dive_site *> divesites);
 	bool editMode() const;
 
 public slots:

map-widget/qmlmapwidgethelper.cpp

@@ -46,18 +46,18 @@ void MapWidgetHelper::centerOnDiveSite(struct dive_site *ds)
 	}
 }
 
-void MapWidgetHelper::setSelected(const QVector<dive_site *> &divesites)
+void MapWidgetHelper::setSelected(const std::vector<dive_site *> divesites)
 {
-	m_mapLocationModel->setSelected(divesites);
+	m_mapLocationModel->setSelected(std::move(divesites));
 	m_mapLocationModel->selectionChanged();
 	updateEditMode();
 }
 
 void MapWidgetHelper::centerOnSelectedDiveSite()
 {
-	QVector<struct dive_site *> selDS = m_mapLocationModel->selectedDs();
+	std::vector<struct dive_site *> selDS = m_mapLocationModel->selectedDs();
 
-	if (selDS.isEmpty()) {
+	if (selDS.empty()) {
 		// no selected dives with GPS coordinates
 		QMetaObject::invokeMethod(m_map, "deselectMapLocation");
 		return;
@@ -128,7 +128,7 @@ void MapWidgetHelper::selectedLocationChanged(struct dive_site *ds_in)
 
 	if (!ds_in)
 		return;
-	MapLocation *location = m_mapLocationModel->getMapLocation(ds_in);
+	const MapLocation *location = m_mapLocationModel->getMapLocation(ds_in);
 	if (!location)
 		return;
 	QGeoCoordinate locationCoord = location->coordinate;

map-widget/qmlmapwidgethelper.h

@@ -36,7 +36,7 @@ public:
 	Q_INVOKABLE void updateCurrentDiveSiteCoordinatesFromMap(struct dive_site *ds, QGeoCoordinate coord);
 	Q_INVOKABLE void selectVisibleLocations();
 	Q_INVOKABLE void selectedLocationChanged(struct dive_site *ds);
-	void setSelected(const QVector<dive_site *> &divesites);
+	void setSelected(const std::vector<dive_site *> divesites);
 	QString pluginObject();
 	bool editMode() const;
 

mobile-widgets/qmlmanager.cpp

@@ -2223,7 +2223,7 @@ void QMLManager::exportToFile(export_types type, QString dir, bool anonymize)
 			break;
 		case EX_DIVE_SITES_XML:
 			{
-				std::vector<const dive_site *> sites = getDiveSitesToExport(false);
+				auto sites = getDiveSitesToExport(false);
 				save_dive_sites_logic(qPrintable(fileName + ".xml"), sites.data(), (int)sites.size(), anonymize);
 				break;
 			}
@@ -2271,7 +2271,7 @@ void QMLManager::shareViaEmail(export_types type, bool anonymize)
 	case EX_DIVE_SITES_XML:
 		fileName.replace("subsurface.log", "subsurface_divesites.xml");
 		{ // need a block so the compiler doesn't complain about creating the sites variable here
-			std::vector<const dive_site *> sites = getDiveSitesToExport(false);
+			auto sites = getDiveSitesToExport(false);
 			if (save_dive_sites_logic(qPrintable(fileName), sites.data(), (int)sites.size(), anonymize) == 0) {
 				// ok, we have a file, let's send it
 				body = "Subsurface dive site data";

qt-models/maplocationmodel.cpp

@@ -4,11 +4,13 @@
 #include "core/divesite.h"
 #include "core/divefilter.h"
 #include "core/divelog.h"
+#include "core/range.h"
 #include "core/settings/qPrefDisplay.h"
 #if !defined(SUBSURFACE_MOBILE) && !defined(SUBSURFACE_DOWNLOADER)
 #include "qt-models/filtermodels.h"
 #include "desktop-widgets/mapwidget.h"
 #endif
+#include <map>
 
 #define MIN_DISTANCE_BETWEEN_DIVE_SITES_M 50.0
 
@@ -78,15 +80,14 @@ MapLocationModel::MapLocationModel(QObject *parent) : QAbstractListModel(parent)
 
 MapLocationModel::~MapLocationModel()
 {
-	qDeleteAll(m_mapLocations);
 }
 
 QVariant MapLocationModel::data(const QModelIndex & index, int role) const
 {
-	if (index.row() < 0 || index.row() >= m_mapLocations.size())
+	if (index.row() < 0 || index.row() >= (int)m_mapLocations.size())
 		return QVariant();
 
-	return m_mapLocations.at(index.row())->getRole(role);
+	return m_mapLocations[index.row()].getRole(role);
 }
 
 QHash<int, QByteArray> MapLocationModel::roleNames() const
@@ -103,17 +104,19 @@ QHash<int, QByteArray> MapLocationModel::roleNames() const
 
 int MapLocationModel::rowCount(const QModelIndex&) const
 {
-	return m_mapLocations.size();
+	return (int)m_mapLocations.size();
 }
 
+/* UNUSED?
 void MapLocationModel::add(MapLocation *location)
 {
-	beginInsertRows(QModelIndex(), m_mapLocations.size(), m_mapLocations.size());
+	beginInsertRows(QModelIndex(), (int)m_mapLocations.size(), (int)m_mapLocations.size());
 	m_mapLocations.append(location);
 	endInsertRows();
 }
+*/
 
-const QVector<dive_site *> &MapLocationModel::selectedDs() const
+const std::vector<dive_site *> &MapLocationModel::selectedDs() const
 {
 	return m_selectedDs;
 }
@@ -132,22 +135,21 @@ static bool hasSelectedDive(const dive_site &ds)
 
 void MapLocationModel::selectionChanged()
 {
-	if (m_mapLocations.isEmpty())
+	if (m_mapLocations.empty())
 		return;
-	for(MapLocation *m: m_mapLocations)
+	for(MapLocation &m: m_mapLocations)
-		m->selected = m_selectedDs.contains(m->divesite);
+		m.selected = range_contains(m_selectedDs, m.divesite);
-	emit dataChanged(createIndex(0, 0), createIndex(m_mapLocations.size() - 1, 0));
+	emit dataChanged(createIndex(0, 0), createIndex((int)m_mapLocations.size() - 1, 0));
 }
 
 void MapLocationModel::reload(QObject *map)
 {
 	beginResetModel();
 
-	qDeleteAll(m_mapLocations);
 	m_mapLocations.clear();
 	m_selectedDs.clear();
 
-	QMap<QString, MapLocation *> locationNameMap;
+	std::map<QString, size_t> locationNameMap;
 
 #if defined(SUBSURFACE_MOBILE) || defined(SUBSURFACE_DOWNLOADER)
 	bool diveSiteMode = false;
@@ -170,7 +172,7 @@ void MapLocationModel::reload(QObject *map)
 			// Dive sites that do not have a gps location are not shown in normal mode.
 			// In dive-edit mode, selected sites are placed at the center of the map,
 			// so that the user can drag them somewhere without having to enter coordinates.
-			if (!diveSiteMode || !m_selectedDs.contains(ds.get()) || !map)
+			if (!diveSiteMode || !range_contains(m_selectedDs, ds.get()) || !map)
 				continue;
 			dsCoord = map->property("center").value<QGeoCoordinate>();
 		} else {
@@ -178,24 +180,24 @@ void MapLocationModel::reload(QObject *map)
 			qreal longitude = ds->location.lon.udeg * 0.000001;
 			dsCoord = QGeoCoordinate(latitude, longitude);
 		}
-		if (!diveSiteMode && hasSelectedDive(*ds) && !m_selectedDs.contains(ds.get()))
+		if (!diveSiteMode && hasSelectedDive(*ds) && !range_contains(m_selectedDs, ds.get()))
-			m_selectedDs.append(ds.get());
+			m_selectedDs.push_back(ds.get());
 		QString name = siteMapDisplayName(ds->name);
 		if (!diveSiteMode) {
 			// don't add dive locations with the same name, unless they are
 			// at least MIN_DISTANCE_BETWEEN_DIVE_SITES_M apart
-			if (locationNameMap.contains(name)) {
+			auto it = locationNameMap.find(name);
-				MapLocation *existingLocation = locationNameMap[name];
+			if (it != locationNameMap.end()) {
-				QGeoCoordinate coord = existingLocation->coordinate;
+				const MapLocation &existingLocation = m_mapLocations[it->second];
+				QGeoCoordinate coord = existingLocation.coordinate;
 				if (dsCoord.distanceTo(coord) < MIN_DISTANCE_BETWEEN_DIVE_SITES_M)
 					continue;
 			}
 		}
-		bool selected = m_selectedDs.contains(ds.get());
+		bool selected = range_contains(m_selectedDs, ds.get());
-		MapLocation *location = new MapLocation(ds.get(), dsCoord, name, selected);
+		m_mapLocations.emplace_back(ds.get(), dsCoord, name, selected);
-		m_mapLocations.append(location);
 		if (!diveSiteMode)
-			locationNameMap[name] = location;
+			locationNameMap[name] = m_mapLocations.size() - 1;
 	}
 
 	endResetModel();
@@ -205,19 +207,19 @@ void MapLocationModel::setSelected(struct dive_site *ds)
 {
 	m_selectedDs.clear();
 	if (ds)
-		m_selectedDs.append(ds);
+		m_selectedDs.push_back(ds);
 }
 
-void MapLocationModel::setSelected(const QVector<dive_site *> &divesites)
+void MapLocationModel::setSelected(const std::vector<dive_site *> &divesites)
 {
 	m_selectedDs = divesites;
 }
 
 MapLocation *MapLocationModel::getMapLocation(const struct dive_site *ds)
 {
-	for (MapLocation *location: m_mapLocations) {
+	for (MapLocation &location: m_mapLocations) {
-		if (ds == location->divesite)
+		if (location.divesite == ds)
-			return location;
+			return &location;
 	}
 	return nullptr;
 }
@@ -225,12 +227,9 @@ MapLocation *MapLocationModel::getMapLocation(const struct dive_site *ds)
 void MapLocationModel::diveSiteChanged(struct dive_site *ds, int field)
 {
 	// Find dive site
-	int row;
+	auto it = std::find_if(m_mapLocations.begin(), m_mapLocations.end(),
-	for (row = 0; row < m_mapLocations.size(); ++row) {
+			       [ds](auto &entry) { return entry.divesite == ds; });
-		if (m_mapLocations[row]->divesite == ds)
+	if (it == m_mapLocations.end())
-			break;
-	}
-	if (row == m_mapLocations.size())
 		return;
 
 	switch (field) {
@@ -239,15 +238,16 @@ void MapLocationModel::diveSiteChanged(struct dive_site *ds, int field)
 			const qreal latitude_r = ds->location.lat.udeg * 0.000001;
 			const qreal longitude_r = ds->location.lon.udeg * 0.000001;
 			QGeoCoordinate coord(latitude_r, longitude_r);
-			m_mapLocations[row]->coordinate = coord;
+			it->coordinate = coord;
 		}
 		break;
 	case LocationInformationModel::NAME:
-		m_mapLocations[row]->name = siteMapDisplayName(ds->name);
+		it->name = siteMapDisplayName(ds->name);
 		break;
 	default:
 		break;
 	}
 
+	int row = static_cast<int>(it - m_mapLocations.begin());
 	emit dataChanged(createIndex(row, 0), createIndex(row, 0));
 }

qt-models/maplocationmodel.h

@@ -3,8 +3,8 @@
 #define MAPLOCATIONMODEL_H
 
 #include "core/subsurface-qt/divelistnotifier.h"
+#include <vector>
 #include <QObject>
-#include <QVector>
 #include <QHash>
 #include <QByteArray>
 #include <QAbstractListModel>
@@ -13,7 +13,7 @@
 class MapLocation
 {
 public:
-	explicit MapLocation(struct dive_site *ds, QGeoCoordinate coord, QString name, bool selected);
+	MapLocation(struct dive_site *ds, QGeoCoordinate coord, QString name, bool selected);
 
 	QVariant getRole(int role) const;
 
@@ -46,9 +46,9 @@ public:
 	// If map is not null, it will be used to place new dive sites without GPS location at the center of the map
 	void reload(QObject *map);
 	void selectionChanged();
-	void setSelected(const QVector<dive_site *> &divesites);
+	void setSelected(const std::vector<dive_site *> &divesites);
-	MapLocation *getMapLocation(const struct dive_site *ds);
+	MapLocation *getMapLocation(const struct dive_site *ds); // Attention: not stable!
-	const QVector<dive_site *> &selectedDs() const;
+	const std::vector<dive_site *> &selectedDs() const;
 	void setSelected(struct dive_site *ds);
 
 protected:
@@ -58,8 +58,8 @@ private slots:
 	void diveSiteChanged(struct dive_site *ds, int field);
 
 private:
-	QVector<MapLocation *> m_mapLocations;
+	std::vector<MapLocation> m_mapLocations;
-	QVector<dive_site *> m_selectedDs;
+	std::vector<dive_site *> m_selectedDs;
 };
 
 #endif