From 05200f9266a4e8e723450abb281c597a48da2e9f Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sun, 22 Sep 2019 14:33:33 +0200 Subject: 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 --- core/btdiscovery.cpp | 14 ---------- core/btdiscovery.h | 5 ++-- core/singleton.h | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 17 deletions(-) create mode 100644 core/singleton.h (limited to 'core') 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 descriptorLookup; namespace { QHash 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 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 #include #include "core/libdivecomputer.h" +#include "core/singleton.h" #if defined(Q_OS_ANDROID) #include @@ -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 { 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 { + * ... + * }; + * 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 +#include + +// 1) Declaration +template +class SillySingleton { + static T *self; +protected: + SillySingleton(); + ~SillySingleton(); +public: + static T *instance(); +}; + +template +T *SillySingleton::self = nullptr; + +// 2) Implementation + +template +SillySingleton::SillySingleton() +{ + if (self) + qWarning("Generating second instance of singleton %s", typeid(T).name()); + self = static_cast(this); + qDebug("Generated singleton %s", typeid(T).name()); +} + +template +SillySingleton::~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 +T *SillySingleton::instance() +{ + if (!self) + new T; + return self; +} + +#endif -- cgit v1.2.3-70-g09d2