Dive computers: turn QMultiMap into sorted vector

The list of known dive computers was stored in a multi-map indexed
by the device name. Turn this into a sorted QVector. Thus, no
map-to-list conversion is needed in the device editing dialog,
which distinctly simplifies the code.

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

core/divecomputer.cpp

@@ -19,6 +19,11 @@ bool DiveComputerNode::operator!=(const DiveComputerNode &a) const
 	return !(*this == a);
 }
 
+bool DiveComputerNode::operator<(const DiveComputerNode &a) const
+{
+	return std::tie(model, deviceId) < std::tie(a.model, a.deviceId);
+}
+
 bool DiveComputerNode::changesValues(const DiveComputerNode &b) const
 {
 	if (model != b.model || deviceId != b.deviceId) {
@@ -32,27 +37,23 @@ bool DiveComputerNode::changesValues(const DiveComputerNode &b) const
 
 const DiveComputerNode *DiveComputerList::getExact(const QString &m, uint32_t d)
 {
-	for (QMap<QString, DiveComputerNode>::iterator it = dcMap.find(m); it != dcMap.end() && it.key() == m; ++it)
-		if (it->deviceId == d)
-			return &*it;
-	return NULL;
+	auto it = std::lower_bound(dcs.begin(), dcs.end(), DiveComputerNode { m, d , {}, {}, {} } );
+	return it != dcs.end() && it->model == m && it->deviceId == d ? &*it : NULL;
 }
 
 const DiveComputerNode *DiveComputerList::get(const QString &m)
 {
-	QMap<QString, DiveComputerNode>::iterator it = dcMap.find(m);
-	if (it != dcMap.end())
-		return &*it;
-	return NULL;
+	auto it = std::lower_bound(dcs.begin(), dcs.end(), DiveComputerNode { m, 0 , {}, {}, {} } );
+	return it != dcs.end() && it->model == m ? &*it : NULL;
 }
 
 void DiveComputerNode::showchanges(const QString &n, const QString &s, const QString &f) const
 {
-	if (nickName != n)
+	if (nickName != n && !n.isEmpty())
 		qDebug("new nickname %s for DC model %s deviceId 0x%x", qPrintable(n), qPrintable(model), deviceId);
-	if (serialNumber != s)
+	if (serialNumber != s && !s.isEmpty())
 		qDebug("new serial number %s for DC model %s deviceId 0x%x", qPrintable(s), qPrintable(model), deviceId);
-	if (firmware != f)
+	if (firmware != f && !f.isEmpty())
 		qDebug("new firmware version %s for DC model %s deviceId 0x%x", qPrintable(f), qPrintable(model), deviceId);
 }
 
@@ -60,28 +61,21 @@ void DiveComputerList::addDC(QString m, uint32_t d, QString n, QString s, QStrin
 {
 	if (m.isEmpty() || d == 0)
 		return;
-	const DiveComputerNode *existNode = this->getExact(m, d);
-
-	if (existNode) {
-		// Update any non-existent fields from the old entry
-		if (n.isEmpty())
-			n = existNode->nickName;
-		if (s.isEmpty())
-			s = existNode->serialNumber;
-		if (f.isEmpty())
-			f = existNode->firmware;
-
-		// Do all the old values match?
-		if (n == existNode->nickName && s == existNode->serialNumber && f == existNode->firmware)
-			return;
-
+	auto it = std::lower_bound(dcs.begin(), dcs.end(), DiveComputerNode { m, d , {}, {}, {} } );
+	if (it != dcs.end() && it->model == m && it->deviceId == d) {
 		// debugging: show changes
 		if (verbose)
-			existNode->showchanges(n, s, f);
-		dcMap.remove(m, *existNode);
+			it->showchanges(n, s, f);
+		// Update any non-existent fields from the old entry
+		if (!n.isEmpty())
+			it->nickName = n;
+		if (!s.isEmpty())
+			it->serialNumber = s;
+		if (!f.isEmpty())
+			it->firmware = f;
+	} else {
+		dcs.insert(it, DiveComputerNode { m, d, s, f, n });
 	}
-
-	dcMap.insert(m, { m, d, s, f, n });
 }
 
 extern "C" void create_device_node(const char *model, uint32_t deviceid, const char *serial, const char *firmware, const char *nickname)
@@ -89,7 +83,7 @@ extern "C" void create_device_node(const char *model, uint32_t deviceid, const c
 	dcList.addDC(model, deviceid, nickname, serial, firmware);
 }
 
-extern "C" bool compareDC(const DiveComputerNode &a, const DiveComputerNode &b)
+static bool compareDCById(const DiveComputerNode &a, const DiveComputerNode &b)
 {
 	return a.deviceId < b.deviceId;
 }
@@ -98,10 +92,9 @@ extern "C" void call_for_each_dc (void *f, void (*callback)(void *, const char *
 							   const char *, const char *, const char *),
 				  bool select_only)
 {
-	QList<DiveComputerNode> values = dcList.dcMap.values();
-	qSort(values.begin(), values.end(), compareDC);
-	for (int i = 0; i < values.size(); i++) {
-		const DiveComputerNode *node = &values.at(i);
+	QVector<DiveComputerNode> values = dcList.dcs;
+	std::sort(values.begin(), values.end(), compareDCById);
+	for (const DiveComputerNode &node: values) {
 		bool found = false;
 		if (select_only) {
 			int j;
@@ -111,7 +104,7 @@ extern "C" void call_for_each_dc (void *f, void (*callback)(void *, const char *
 				if (!d->selected)
 					continue;
 				for_each_dc(d, dc) {
-					if (dc->deviceid == node->deviceId) {
+					if (dc->deviceid == node.deviceId) {
 						found = true;
 						break;
 					}
@@ -123,12 +116,11 @@ extern "C" void call_for_each_dc (void *f, void (*callback)(void *, const char *
 			found = true;
 		}
 		if (found)
-			callback(f, qPrintable(node->model), node->deviceId, qPrintable(node->nickName),
-				 qPrintable(node->serialNumber), qPrintable(node->firmware));
+			callback(f, qPrintable(node.model), node.deviceId, qPrintable(node.nickName),
+				 qPrintable(node.serialNumber), qPrintable(node.firmware));
 	}
 }
 
-
 extern "C" int is_default_dive_computer(const char *vendor, const char *product)
 {
 	auto dc = SettingsObjectWrapper::instance()->dive_computer_settings;

core/divecomputer.h

@@ -3,13 +3,14 @@
 #define DIVECOMPUTER_H
 
 #include <QString>
-#include <QMap>
+#include <QVector>
 #include <stdint.h>
 
 class DiveComputerNode {
 public:
 	bool operator==(const DiveComputerNode &a) const;
 	bool operator!=(const DiveComputerNode &a) const;
+	bool operator<(const DiveComputerNode &a) const;
 	bool changesValues(const DiveComputerNode &b) const;
 	void showchanges(const QString &n, const QString &s, const QString &f) const;
 	QString model;
@@ -26,7 +27,9 @@ public:
 	void addDC(QString m, uint32_t d, QString n = QString(), QString s = QString(), QString f = QString());
 	DiveComputerNode matchDC(const QString &m, uint32_t d);
 	DiveComputerNode matchModel(const QString &m);
-	QMultiMap<QString, DiveComputerNode> dcMap;
+
+	// Keep the dive computers in a vector sorted by (model, deviceId)
+	QVector<DiveComputerNode> dcs;
 };
 
 extern DiveComputerList dcList;

desktop-widgets/divecomputermanagementdialog.cpp

@@ -19,8 +19,7 @@ DiveComputerManagementDialog::DiveComputerManagementDialog(QWidget *parent, Qt::
 
 void DiveComputerManagementDialog::init()
 {
-	model.reset(new DiveComputerModel(dcList.dcMap));
-	model->update();
+	model.reset(new DiveComputerModel);
 	ui.tableView->setModel(model.data());
 	ui.tableView->resizeColumnsToContents();
 	ui.tableView->setColumnWidth(DiveComputerModel::REMOVE, 22);
@@ -57,7 +56,6 @@ void DiveComputerManagementDialog::accept()
 
 void DiveComputerManagementDialog::reject()
 {
-	model->dropWorkingList();
 	hide();
 	close();
 }

desktop-widgets/mainwindow.cpp

@@ -770,7 +770,7 @@ void MainWindow::closeCurrentFile()
 
 	clear_events();
 
-	dcList.dcMap.clear();
+	dcList.dcs.clear();
 }
 
 void MainWindow::updateCloudOnlineStatus()

qt-models/divecomputermodel.cpp

@@ -3,70 +3,45 @@
 #include "core/dive.h"
 #include "core/divelist.h"
 
-DiveComputerModel::DiveComputerModel(QMultiMap<QString, DiveComputerNode> &dcMap, QObject *parent) : CleanerTableModel(parent)
+DiveComputerModel::DiveComputerModel(QObject *parent) : CleanerTableModel(parent),
+	dcs(dcList.dcs)
 {
 	setHeaderDataStrings(QStringList() << "" << tr("Model") << tr("Device ID") << tr("Nickname"));
-	dcWorkingMap = dcMap;
-	numRows = 0;
 }
 
 QVariant DiveComputerModel::data(const QModelIndex &index, int role) const
 {
-	QList<DiveComputerNode> values = dcWorkingMap.values();
-	DiveComputerNode node = values.at(index.row());
+	if (index.row() < 0 || index.row() >= dcs.size())
+		return QVariant();
+	const DiveComputerNode &node = dcs[index.row()];
 
-	QVariant ret;
 	if (role == Qt::DisplayRole || role == Qt::EditRole) {
 		switch (index.column()) {
 		case ID:
-			ret = QString("0x").append(QString::number(node.deviceId, 16));
-			break;
+			return QString("0x").append(QString::number(node.deviceId, 16));
 		case MODEL:
-			ret = node.model;
-			break;
+			return node.model;
 		case NICKNAME:
-			ret = node.nickName;
-			break;
+			return node.nickName;
 		}
 	}
 
 	if (index.column() == REMOVE) {
 		switch (role) {
 		case Qt::DecorationRole:
-			ret = trashIcon();
-			break;
+			return trashIcon();
 		case Qt::SizeHintRole:
-			ret = trashIcon().size();
-			break;
+			return trashIcon().size();
 		case Qt::ToolTipRole:
-			ret = tr("Clicking here will remove this dive computer.");
-			break;
+			return tr("Clicking here will remove this dive computer.");
 		}
 	}
-	return ret;
+	return QVariant();
 }
 
 int DiveComputerModel::rowCount(const QModelIndex&) const
 {
-	return numRows;
-}
-
-void DiveComputerModel::update()
-{
-	QList<DiveComputerNode> values = dcWorkingMap.values();
-	int count = values.count();
-
-	if (numRows) {
-		beginRemoveRows(QModelIndex(), 0, numRows - 1);
-		numRows = 0;
-		endRemoveRows();
-	}
-
-	if (count) {
-		beginInsertRows(QModelIndex(), 0, count - 1);
-		numRows = count;
-		endInsertRows();
-	}
+	return dcs.size();
 }
 
 Qt::ItemFlags DiveComputerModel::flags(const QModelIndex &index) const
@@ -80,33 +55,27 @@ Qt::ItemFlags DiveComputerModel::flags(const QModelIndex &index) const
 bool DiveComputerModel::setData(const QModelIndex &index, const QVariant &value, int)
 {
 	// We should test if the role == Qt::EditRole
+	if (index.row() < 0 || index.row() >= dcs.size())
+		return false;
 
-	// WARN: This seems wrong - The values don't are ordered - we need a map from the Key to Index, or something.
-	QList<DiveComputerNode> values = dcWorkingMap.values();
-	DiveComputerNode node = values.at(index.row());
-	dcWorkingMap.remove(node.model, node);
+	DiveComputerNode &node = dcs[index.row()];
 	node.nickName = value.toString();
-	dcWorkingMap.insert(node.model, node);
 	emit dataChanged(index, index);
 	return true;
 }
 
 void DiveComputerModel::remove(const QModelIndex &index)
 {
-	QList<DiveComputerNode> values = dcWorkingMap.values();
-	DiveComputerNode node = values.at(index.row());
-	dcWorkingMap.remove(node.model, node);
-	update();
-}
-
-void DiveComputerModel::dropWorkingList()
-{
-	// how do I prevent the memory leak ?
+	if (index.row() < 0 || index.row() >= dcs.size())
+		return;
+	beginRemoveRows(QModelIndex(), index.row(), index.row());
+	dcs.remove(index.row());
+	endRemoveRows();
 }
 
 void DiveComputerModel::keepWorkingList()
 {
-	if (dcList.dcMap != dcWorkingMap)
+	if (dcList.dcs != dcs)
 		mark_divelist_changed(true);
-	dcList.dcMap = dcWorkingMap;
+	dcList.dcs = dcs;
 }

qt-models/divecomputermodel.h

@@ -14,14 +14,12 @@ public:
 		ID,
 		NICKNAME
 	};
-	DiveComputerModel(QMultiMap<QString, DiveComputerNode> &dcMap, QObject *parent = 0);
+	DiveComputerModel(QObject *parent = 0);
 	virtual QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const;
 	virtual int rowCount(const QModelIndex &parent = QModelIndex()) const;
 	virtual Qt::ItemFlags flags(const QModelIndex &index) const;
 	virtual bool setData(const QModelIndex &index, const QVariant &value, int role = Qt::EditRole);
-	void update();
 	void keepWorkingList();
-	void dropWorkingList();
 
 public
 slots:
@@ -29,7 +27,7 @@ slots:
 
 private:
 	int numRows;
-	QMultiMap<QString, DiveComputerNode> dcWorkingMap;
+	QVector<DiveComputerNode> dcs;
 };
 
 #endif