Divecomputer nickname handling..

[Linus Torvalds]

Ok, this is almost certainly entirely broken, because all I tested was 
that it compiles for me. I'm not a huge fan of C++ ("really? Please don't 
tell us again"), and I hate how I can't split up member functions into 
static helper functions, so I got - once again - disgusted when I had to 
add a "showchanges" member for no actual good reason just because I wanted 
to split things up and make this more readable.

I also didn't want to change the prototype fro the "AddDC" member, but I 
wanted to change the use of the variables from the "constant pass by 
reference" to actual local variables that I can change, so I did that 
insane dance with variables there too.

In other words, you should not apply this. You should probably go "Linus 
is insane, and this is not good, but the *right* way to do it is Xyz", and 
just do Xyz instead.

For example, another alternative that I considered (and maybe should have 
done) was to just create member functions to update the serial number, the 
firmware version and the nickname. And then AddDC() would just do 
something like

 (a) look up old entry with matching model/deviceid information.
 (b) if no old entry exists, create a new entry with empty information 
     and insert that
 (c) just call the update functions for the passed-in firmware/serial/ 
     nickname.

that would have been closer to what we used to do.

This patch seemed closer to the new model, but tries to just make sure 
that if we pass in empty firmware/serial/nickname information, we take the 
values from whatever old entry we find instead.

And I *really* didn't test this. Yeah, I created a nickname for my EON, 
and then I tried to download, and the nickname stayed around, but I didn't 
actually test that that failed before, so the "test" is fundamentally 
flawed crap.

Also, to be honest, I did a "force download all dives", and then clicked 
"ok", and now I have a couple of dives that have *two* EON Steel cases, 
apparently because we merged things and didn't notice we already had the 
first. That seems to be a separate issue entirely, though - the ones that 
merged correctly had the original EON Steel import as the first one.

I'll have to think more about that merging issue, but I'm pretty sure it 
has nothing to do with this patch.

Anyway, despite the crap testing, this *looks* like it might work. That 
must count for something? I mean, it did actually compile, so it must 
be perfect ...

Not-Yet-Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
---

You can remove the "Not-Yet-" part of my signoff if you actually review 
this.  Or just consider it a failed attempt if you want to do the other 
approach.

 divecomputer.cpp | 47 ++++++++++++++++++++++++++++++++---------------
 divecomputer.h   |  1 +
 2 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/divecomputer.cpp b/divecomputer.cpp
index b45b74a449b9..a487578e7144 100644
--- a/divecomputer.cpp
+++ b/divecomputer.cpp
@@ -58,26 +58,43 @@ const DiveComputerNode *DiveComputerList::get(const QString &m)
 	return NULL;
 }
 
-void DiveComputerList::addDC(const QString &m, uint32_t d, const QString &n, const QString &s, const QString &f)
+void DiveComputerNode::showchanges(const QString &n, const QString &s, const QString &f) const
+{
+	if (nickName != n)
+		qDebug("new nickname %s for DC model %s deviceId 0x%x", n.toUtf8().data(), model.toUtf8().data(), deviceId);
+	if (serialNumber != s)
+		qDebug("new serial number %s for DC model %s deviceId 0x%x", s.toUtf8().data(), model.toUtf8().data(), deviceId);
+	if (firmware != f)
+		qDebug("new firmware version %s for DC model %s deviceId 0x%x", f.toUtf8().data(), model.toUtf8().data(), deviceId);
+}
+
+void DiveComputerList::addDC(const QString &m, uint32_t d, const QString &_n, const QString &_s, const QString &_f)
 {
 	if (m.isEmpty() || d == 0)
 		return;
-	const DiveComputerNode *existNode = this->getExact(m, d);
-	DiveComputerNode newNode(m, d, s, f, n);
-	if (existNode) {
-		if (newNode.changesValues(*existNode)) {
-			if (n.size() && existNode->nickName != n)
-				qDebug("new nickname %s for DC model %s deviceId 0x%x", n.toUtf8().data(), m.toUtf8().data(), d);
-			if (f.size() && existNode->firmware != f)
-				qDebug("new firmware version %s for DC model %s deviceId 0x%x", f.toUtf8().data(), m.toUtf8().data(), d);
-			if (s.size() && existNode->serialNumber != s)
-				qDebug("new serial number %s for DC model %s deviceId 0x%x", s.toUtf8().data(), m.toUtf8().data(), d);
-		} else {
+	const DiveComputerNode *Old = this->getExact(m, d);
+	QString n = _n, s = _s, f = _f;
+
+	if (Old) {
+		// Update any non-existent fields from the old entry
+		if (!n.size())
+			n = Old->nickName;
+		if (!s.size())
+			s = Old->serialNumber;
+		if (!f.size())
+			f = Old->firmware;
+
+		// Do all the old values match?
+		if (n == Old->nickName && s == Old->serialNumber && f == Old->firmware)
 			return;
-		}
-		dcMap.remove(m, *existNode);
+
+		// debugging: show changes
+		Old->showchanges(n, s, f);
+		dcMap.remove(m, *Old);
 	}
-	dcMap.insert(m, newNode);
+
+	DiveComputerNode New(m, d, s, f, n);
+	dcMap.insert(m, New);
 }
 
 extern "C" void create_device_node(const char *model, uint32_t deviceid, const char *serial, const char *firmware, const char *nickname)
diff --git a/divecomputer.h b/divecomputer.h
index 81399c47d84d..28785dd60e00 100644
--- a/divecomputer.h
+++ b/divecomputer.h
@@ -12,6 +12,7 @@ public:
 	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;
 	uint32_t deviceId;
 	QString serialNumber;