Merge pull request #66 from SLM-Audio/syl/refactor-database-property-listener

rename to DatabasePropertyWatcher, return unique_ptr instead of optional

Author: MeijisIrlnd

include/mostly_harmless/CMakeLists.txt

@@ -27,6 +27,6 @@ set(MOSTLYHARMLESS_HEADERS
         ${CMAKE_CURRENT_SOURCE_DIR}/utils/mostlyharmless_Proxy.h
         ${CMAKE_CURRENT_SOURCE_DIR}/utils/mostlyharmless_NoDenormals.h
         ${CMAKE_CURRENT_SOURCE_DIR}/data/mostlyharmless_DatabaseState.h
-		${CMAKE_CURRENT_SOURCE_DIR}/data/mostlyharmless_DatabasePropertyListener.h
+        ${CMAKE_CURRENT_SOURCE_DIR}/data/mostlyharmless_DatabasePropertyWatcher.h
         ${PLATFORM_HEADERS}
         PARENT_SCOPE)

…ostlyharmless_DatabasePropertyListener.h …mostlyharmless_DatabasePropertyWatcher.hinclude/mostly_harmless/data/mostlyharmless_DatabasePropertyListener.h renamed to include/mostly_harmless/data/mostlyharmless_DatabasePropertyWatcher.h include/mostly_harmless/data/mostlyharmless_DatabasePropertyListener.h renamed to include/mostly_harmless/data/mostlyharmless_DatabasePropertyWatcher.h

@@ -2,8 +2,8 @@
 // Created by Syl Morrison on 13/04/2025.
 //
 
-#ifndef MOSTLYHARMLESS_DATABASEPROPERTYLISTENER_H
-#define MOSTLYHARMLESS_DATABASEPROPERTYLISTENER_H
+#ifndef MOSTLYHARMLESS_DATABASEPROPERTYWATCHER_H
+#define MOSTLYHARMLESS_DATABASEPROPERTYWATCHER_H
 #include <mostly_harmless/data/mostlyharmless_DatabaseState.h>
 #include <mostly_harmless/mostlyharmless_Concepts.h>
 #include <mostly_harmless/utils/mostlyharmless_Timer.h>
@@ -16,50 +16,70 @@ namespace mostly_harmless::data {
      * @tparam T Any type satisfying DatabaseStorageType
      */
     template <DatabaseStorageType T>
-    class DatabasePropertyListener final {
+    class DatabasePropertyWatcher final {
     private:
         struct Private {};
 
     public:
         /**
          * @private
          */
-        DatabasePropertyListener(Private, DatabaseState&& databaseState, std::string_view name, int pollFrequencyMs, std::function<void(const T&)>&& callback) : m_databaseState(std::move(databaseState)),
-                                                                                                                                                                 m_propertyNameToWatch(name),
-                                                                                                                                                                 m_callback(callback) {
-            m_proxyThis = utils::Proxy<DatabasePropertyListener<T>>::create(this);
+        DatabasePropertyWatcher(Private, DatabaseState&& databaseState, std::string_view name, int pollFrequencyMs, std::function<void(const T&)>&& callback) : m_databaseState(std::move(databaseState)),
+                                                                                                                                                                m_propertyNameToWatch(name),
+                                                                                                                                                                m_pollFrequencyMs(pollFrequencyMs),
+                                                                                                                                                                m_callback(std::move(callback)) {
+            m_proxyThis = utils::Proxy<DatabasePropertyWatcher<T>>::create(this);
             auto proxyCopy = m_proxyThis;
             m_timer.action = [this, proxyCopy]() -> void {
                 if (!proxyCopy->isValid()) {
                     return;
                 }
                 timerCallback();
             };
-            m_timer.run(pollFrequencyMs);
+            m_timer.run(m_pollFrequencyMs);
         }
 
+        /**
+         * Non copyable
+         */
+        DatabasePropertyWatcher(const DatabasePropertyWatcher& /*other*/) = delete;
+        /**
+         * Non moveable
+         */
+        DatabasePropertyWatcher(DatabasePropertyWatcher&& /*other*/) noexcept = delete;
+
         /**
          * @private
          */
-        ~DatabasePropertyListener() noexcept {
+        ~DatabasePropertyWatcher() noexcept {
             m_proxyThis->null();
         }
 
         /**
-         * Attempts to create a DatabasePropertyListener for the given property name.
+         * Non copyable
+         */
+        auto operator=(const DatabasePropertyWatcher& /*other*/) -> DatabasePropertyWatcher = delete;
+
+        /**
+         * Non moveable
+         */
+        auto operator=(DatabasePropertyWatcher&& /*other*/) noexcept -> DatabasePropertyWatcher = delete;
+
+        /**
+         * Attempts to create a DatabasePropertyWatcher for the given property name.
          * As sqlite requires database access across different processes to each have their own unique connection, we first call `DatabaseState::duplicate` to create a new connection specifically for this thread.
-         * @param databaseState A const reference to the database connection to duplicate for this PropertyListener.
+         * @param databaseState A const reference to the database connection to duplicate for this PropertyWatcher.
          * @param propertyName The name of the property to watch.
          * @param pollFrequencyMs The interval in milliseconds to poll the database for changes at.
          * @param callback A lambda to be invoked from a background timer thread on property value change.
-         * @return A DatabasePropertyListener for the given property name if successful, nullopt otherwise.
+         * @return A DatabasePropertyWatcher for the given property name if successful, nullptr otherwise.
          */
-        static auto tryCreate(const DatabaseState& databaseState, std::string_view propertyName, int pollFrequencyMs, std::function<void(const T&)>&& callback) -> std::optional<DatabasePropertyListener> {
+        static auto tryCreate(const DatabaseState& databaseState, std::string_view propertyName, int pollFrequencyMs, std::function<void(const T&)>&& callback) -> std::unique_ptr<DatabasePropertyWatcher> {
             auto databaseCopyOpt = databaseState.duplicate();
             if (!databaseCopyOpt) {
-                return {};
+                return nullptr;
             }
-            return std::make_optional<DatabasePropertyListener<T>>(Private{}, std::move(*databaseCopyOpt), propertyName, pollFrequencyMs, std::move(callback));
+            return std::make_unique<DatabasePropertyWatcher<T>>(Private{}, std::move(*databaseCopyOpt), propertyName, pollFrequencyMs, std::move(callback));
         }
 
 
@@ -73,12 +93,7 @@ namespace mostly_harmless::data {
                 assert(false);
                 return;
             }
-            auto hasChanged{ false };
-            if (!m_previousValue.has_value()) {
-                hasChanged = true;
-            } else {
-                hasChanged = *m_previousValue != *getRes;
-            }
+            const auto hasChanged = m_previousValue != getRes;
             m_previousValue = *getRes;
             if (!hasChanged) {
                 return;
@@ -88,10 +103,11 @@ namespace mostly_harmless::data {
 
         DatabaseState m_databaseState;
         std::string m_propertyNameToWatch;
+        int m_pollFrequencyMs;
         std::function<void(const T&)> m_callback;
-        std::shared_ptr<utils::Proxy<DatabasePropertyListener<T>>> m_proxyThis;
+        std::shared_ptr<utils::Proxy<DatabasePropertyWatcher<T>>> m_proxyThis;
         utils::Timer m_timer;
         std::optional<T> m_previousValue{};
     };
 } // namespace mostly_harmless::data
-#endif // MOSTLYHARMLESS_DATABASEPROPERTYLISTENER_H
+#endif // MOSTLYHARMLESS_DATABASEPROPERTYWATCHER_H

source/CMakeLists.txt

@@ -33,7 +33,7 @@ set(MOSTLYHARMLESS_SOURCES
         ${CMAKE_CURRENT_SOURCE_DIR}/utils/mostlyharmless_Hash.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/utils/mostlyharmless_Macros.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/data/mostlyharmless_DatabaseState.cpp
-        ${CMAKE_CURRENT_SOURCE_DIR}/data/mostlyharmless_DatabasePropertyListener.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/data/mostlyharmless_DatabasePropertyWatcher.cpp
         ${PLATFORM_SOURCES}
         PARENT_SCOPE)
 message(STATUS ${MOSTLYHARMLESS_PLATFORM_SOURCES})

source/data/mostlyharmless_DatabasePropertyListener.cpp

@@ -0,0 +1,4 @@
+//
+// Created by Syl Morrison on 13/04/2025.
+//
+#include <mostly_harmless/data/mostlyharmless_DatabasePropertyWatcher.h>

source/data/mostlyharmless_DatabasePropertyWatcher.cpp

@@ -3,7 +3,7 @@
 //
 #include <mostly_harmless/utils/mostlyharmless_Directories.h>
 #include <mostly_harmless/data/mostlyharmless_DatabaseState.h>
-#include <mostly_harmless/data/mostlyharmless_DatabasePropertyListener.h>
+#include <mostly_harmless/data/mostlyharmless_DatabasePropertyWatcher.h>
 #include <catch2/catch_test_macros.hpp>
 #include <catch2/matchers/catch_matchers_floating_point.hpp>
 
@@ -59,9 +59,9 @@ namespace mostly_harmless::testing {
             std::filesystem::remove(dbFile);
         }
 
-//        SECTION("Test Invalid Location") {
-//            tryCreateDatabase<false>("INVALID LOCATION", {});
-//        }
+        SECTION("Test Invalid Location") {
+            tryCreateDatabase<false>("INVALID LOCATION", {});
+        }
 
         SECTION("Test In-Memory") {
             tryCreateDatabase<true>(":memory:", {});
@@ -81,7 +81,7 @@ namespace mostly_harmless::testing {
             std::filesystem::remove(dbFile);
         }
 
-        SECTION("Test DatabasePropertyListener") {
+        SECTION("Test DatabasePropertyWatcher") {
             for (auto i = 0; i < 100; ++i) {
                 {
                     auto databaseOpt = tryCreateDatabase<true>(dbFile, { { "test", 0 } });
@@ -93,7 +93,7 @@ namespace mostly_harmless::testing {
                             wasPropertyChanged.store(true);
                             newValue = x;
                         };
-                        auto listener = data::DatabasePropertyListener<int>::tryCreate(database, "test", 1, std::move(onPropertyChanged));
+                        auto listener = data::DatabasePropertyWatcher<int>::tryCreate(database, "test", 1, std::move(onPropertyChanged));
                         REQUIRE(listener);
                         database.set<int>("test", 10);
                         std::this_thread::sleep_for(std::chrono::milliseconds(50));