Cleanup: unify idiosyncratic singletons

The way we handle singletons in QML, QML insists on allocating the
objects. This leads to a very idiosyncratic way of handling
singletons: The global instance pointer is set in the constructor.

Unify all these by implementing a "SillySingleton" template. All
of the weird singleton-classes can derive from this template and
don't have to bother with reimplementing the instance() function
with all the safety-checks, etc.

This serves firstly as documentation but also improves debugging
as we will now see wanted and unwanted creation and destruction
of these weird singletons.

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

3 files changed, 76 insertions, 17 deletions

diff --git a/core/btdiscovery.cpp b/core/btdiscovery.cpp
index 8db1e02cc..1387be400 100644
--- a/core/btdiscovery.cpp
+++ b/core/btdiscovery.cpp
@@ -15,7 +15,6 @@ extern QMap<QString, dc_descriptor_t *> descriptorLookup;
 namespace {
 	QHash<QString, QBluetoothDeviceInfo> btDeviceInfo;
 }
-BTDiscovery *BTDiscovery::m_instance = NULL;
 
 static dc_descriptor_t *getDeviceType(QString btName)
 // central function to convert a BT name to a Subsurface known vendor/model pair
@@ -139,11 +138,6 @@ BTDiscovery::BTDiscovery(QObject*) : m_btValid(false),
 	m_showNonDiveComputers(false),
 	discoveryAgent(nullptr)
 {
-	if (m_instance) {
-		qDebug() << "trying to create an additional BTDiscovery object";
-		return;
-	}
-	m_instance = this;
 #if defined(BT_SUPPORT)
 	QLoggingCategory::setFilterRules(QStringLiteral("qt.bluetooth* = true"));
 	BTDiscoveryReDiscover();
@@ -202,19 +196,11 @@ void BTDiscovery::BTDiscoveryReDiscover()
 
 BTDiscovery::~BTDiscovery()
 {
-	m_instance = NULL;
 #if defined(BT_SUPPORT)
 	delete discoveryAgent;
 #endif
 }
 
-BTDiscovery *BTDiscovery::instance()
-{
-	if (!m_instance)
-		m_instance = new BTDiscovery();
-	return m_instance;
-}
-
 #if defined(BT_SUPPORT)
 extern void addBtUuid(QBluetoothUuid uuid);
 extern QHash<QString, QStringList> productList;
diff --git a/core/btdiscovery.h b/core/btdiscovery.h
index d6c25d8b0..a7d68decd 100644
--- a/core/btdiscovery.h
+++ b/core/btdiscovery.h
@@ -10,6 +10,7 @@
 #include <QBluetoothDeviceDiscoveryAgent>
 #include <QBluetoothUuid>
 #include "core/libdivecomputer.h"
+#include "core/singleton.h"
 
 #if defined(Q_OS_ANDROID)
 #include <QAndroidJniObject>
@@ -23,13 +24,12 @@ QString extractBluetoothAddress(const QString &address);
 QString extractBluetoothNameAddress(const QString &address, QString &name);
 QBluetoothDeviceInfo getBtDeviceInfo(const QString &devaddr);
 
-class BTDiscovery : public QObject {
+class BTDiscovery : public QObject, public SillySingleton<BTDiscovery> {
 	Q_OBJECT
 
 public:
 	BTDiscovery(QObject *parent = NULL);
 	~BTDiscovery();
-	static BTDiscovery *instance();
 
 	struct btPairedDevice {
 		QString address;
@@ -57,7 +57,6 @@ public:
 	void discoverAddress(QString address);
 
 private:
-	static BTDiscovery *m_instance;
 	bool m_btValid;
 	bool m_showNonDiveComputers;
 
diff --git a/core/singleton.h b/core/singleton.h
new file mode 100644
index 000000000..9573b07e8
--- /dev/null
+++ b/core/singleton.h
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * We use singletons in numerous places. In combination with QML this gives
+ * us a very fundamental problem, because QML likes to allocate objects by
+ * itself. There are known workarounds, but currently we use idiosyncratic
+ * singleton classes, which initialize the global instance pointer in the
+ * constructor. Things get even more complicated if the same singleton is used
+ * on mobile and on desktop. The latter might want to simply use the classical
+ * instance() function without having to initialize the singleton first, as
+ * that would beat the purpose of the instance() method.
+ *
+ * The template defined here, SillySingleton, codifies all this. Simply derive
+ * a class from this template:
+ *	class X : public SillySingleton<X> {
+ *		...
+ *	};
+ * This will generate an instance() method. This will do the right thing for
+ * both methods: explicit construction of the singleton via new or implicit
+ * by calling the instance() method. It will also generate warnings if a
+ * singleton class is generated more than once (i.e. first instance() is called
+ * and _then_ new).
+ *
+ * In the long run we should get rid of all users of this class.
+ */
+#ifndef SINGLETON_H
+#define SINGLETON_H
+
+#include <typeinfo>
+#include <QtGlobal>
+
+// 1) Declaration
+template<typename T>
+class SillySingleton {
+	static T *self;
+protected:
+	SillySingleton();
+	~SillySingleton();
+public:
+	static T *instance();
+};
+
+template<typename T>
+T *SillySingleton<T>::self = nullptr;
+
+// 2) Implementation
+
+template<typename T>
+SillySingleton<T>::SillySingleton()
+{
+	if (self)
+		qWarning("Generating second instance of singleton %s", typeid(T).name());
+	self = static_cast<T *>(this);
+	qDebug("Generated singleton %s", typeid(T).name());
+}
+
+template<typename T>
+SillySingleton<T>::~SillySingleton()
+{
+	if (self == this)
+		self = nullptr;
+	else
+		qWarning("Destroying unknown instance of singleton %s", typeid(T).name());
+	qDebug("Destroyed singleton %s", typeid(T).name());
+}
+
+template<typename T>
+T *SillySingleton<T>::instance()
+{
+	if (!self)
+		new T;
+	return self;
+}
+
+#endif