From fe953f25d83d29f2bdb2b4ed3e490e81472ccff3 Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Fri, 5 Jan 2024 11:08:12 +0100 Subject: [PATCH] Incorporate feedback from @MiKom - Add license header to connection_evaluator.h - Use slotInvocation instead of Connection in the ConnectionEvaluator - Add warnings that deferred connections are experimental - Improved documentation for deferred connections --- src/kdbindings/connection_evaluator.h | 46 +++++++++++++++++---------- src/kdbindings/connection_handle.h | 1 + src/kdbindings/genindex_array.h | 6 ++-- src/kdbindings/signal.h | 8 +++-- 4 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/kdbindings/connection_evaluator.h b/src/kdbindings/connection_evaluator.h index c7e9399..184644e 100644 --- a/src/kdbindings/connection_evaluator.h +++ b/src/kdbindings/connection_evaluator.h @@ -1,7 +1,16 @@ +/* + This file is part of KDBindings. + + SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company + Author: Shivam Kunwar + + SPDX-License-Identifier: MIT + + Contact KDAB at for commercial licensing options. +*/ #pragma once #include -#include #include #include @@ -11,6 +20,8 @@ namespace KDBindings { /** * @brief Manages and evaluates deferred Signal connections. * + * @warning Deferred connections are experimental and may be removed or changed in the future. + * * The ConnectionEvaluator class is responsible for managing and evaluating connections * to Signals. It provides mechanisms to delay and control the evaluation of connections. * It therefore allows controlling when and on which thread slots connected to a Signal are executed. @@ -21,16 +32,19 @@ class ConnectionEvaluator { public: + /** ConnectionEvaluators are default constructible */ ConnectionEvaluator() = default; - // ConnectionEvaluator is not copyable, as it is designed to manage connections, + /** Connectionevaluators are not copyable */ + // As it is designed to manage connections, // and copying it could lead to unexpected behavior, including duplication of connections and issues // related to connection lifetimes. Therefore, it is intentionally made non-copyable. ConnectionEvaluator(const ConnectionEvaluator &) noexcept = delete; ConnectionEvaluator &operator=(const ConnectionEvaluator &) noexcept = delete; - // ConnectionEvaluators are not moveable, as they are captures by-reference + /** ConnectionEvaluators are not moveable */ + // As they are captures by-reference // by the Signal, so moving them would lead to a dangling reference. ConnectionEvaluator(ConnectionEvaluator &&other) noexcept = delete; @@ -40,43 +54,43 @@ class ConnectionEvaluator * @brief Evaluate the deferred connections. * * This function is responsible for evaluating and executing deferred connections. - * And this function ensures thread safety + * This function is thread safe. */ void evaluateDeferredConnections() { - std::lock_guard lock(m_connectionsMutex); + std::lock_guard lock(m_slotInvocationMutex); - for (auto &pair : m_deferredConnections) { + for (auto &pair : m_deferredSlotInvocations) { pair.second(); } - m_deferredConnections.clear(); + m_deferredSlotInvocations.clear(); } private: template friend class Signal; - void enqueueSlotInvocation(const ConnectionHandle &handle, const std::function &connection) + void enqueueSlotInvocation(const ConnectionHandle &handle, const std::function &slotInvocation) { - std::lock_guard lock(m_connectionsMutex); - m_deferredConnections.push_back({ handle, std::move(connection) }); + std::lock_guard lock(m_slotInvocationMutex); + m_deferredSlotInvocations.push_back({ handle, std::move(slotInvocation) }); } void dequeueSlotInvocation(const ConnectionHandle &handle) { - std::lock_guard lock(m_connectionsMutex); + std::lock_guard lock(m_slotInvocationMutex); auto handleMatches = [&handle](const auto &invocationPair) { return invocationPair.first == handle; }; // Remove all invocations that match the handle - m_deferredConnections.erase( - std::remove_if(m_deferredConnections.begin(), m_deferredConnections.end(), handleMatches), - m_deferredConnections.end()); + m_deferredSlotInvocations.erase( + std::remove_if(m_deferredSlotInvocations.begin(), m_deferredSlotInvocations.end(), handleMatches), + m_deferredSlotInvocations.end()); } - std::vector>> m_deferredConnections; - std::mutex m_connectionsMutex; + std::vector>> m_deferredSlotInvocations; + std::mutex m_slotInvocationMutex; }; } // namespace KDBindings diff --git a/src/kdbindings/connection_handle.h b/src/kdbindings/connection_handle.h index 7a0c728..247c9d5 100644 --- a/src/kdbindings/connection_handle.h +++ b/src/kdbindings/connection_handle.h @@ -11,6 +11,7 @@ #include #include +#include namespace KDBindings { diff --git a/src/kdbindings/genindex_array.h b/src/kdbindings/genindex_array.h index 9205d11..4da17d8 100644 --- a/src/kdbindings/genindex_array.h +++ b/src/kdbindings/genindex_array.h @@ -37,8 +37,8 @@ struct GenerationalIndex { uint32_t index = 0; uint32_t generation = 0; - bool operator==(const GenerationalIndex& rhs) const { - // Include both index and generation in the equality check + bool operator==(const GenerationalIndex &rhs) const + { return index == rhs.index && generation == rhs.generation; } }; @@ -207,6 +207,6 @@ class GenerationalIndexArray } }; -} //namespace Private +} // namespace Private } // namespace KDBindings diff --git a/src/kdbindings/signal.h b/src/kdbindings/signal.h index 0fb5a6f..44e0121 100644 --- a/src/kdbindings/signal.h +++ b/src/kdbindings/signal.h @@ -263,17 +263,19 @@ class Signal /** * @brief Establishes a deferred connection between the provided evaluator and slot. * + * @warning Deferred connections are experimental and may be removed or changed in the future. + * * This function allows connecting an evaluator and a slot such that the slot's execution * is deferred until the conditions evaluated by the `evaluator` are met. * - * First argument to the function is reference to a shared pointer to the `ConnectionEvaluator` responsible for determining + * First argument to the function is reference to a shared pointer to the ConnectionEvaluator responsible for determining * when the slot should be executed. * * @return An instance of ConnectionHandle, that can be used to disconnect * or temporarily block the connection. * * @note - * The `KDBindings::Signal` class itself is not thread-safe. While the `ConnectionEvaluator` is inherently + * The Signal class itself is not thread-safe. While the ConnectionEvaluator is inherently * thread-safe, ensure that any concurrent access to this Signal is protected externally to maintain thread safety. */ ConnectionHandle connectDeferred(const std::shared_ptr &evaluator, std::function const &slot) @@ -382,7 +384,7 @@ class Signal */ bool blockConnection(const ConnectionHandle &handle, bool blocked) { - if (m_impl && handle.belongsTo(*this) && handle.m_id.has_value() ) { + if (m_impl && handle.belongsTo(*this) && handle.m_id.has_value()) { return m_impl->blockConnection(*handle.m_id, blocked); } else { throw std::out_of_range("Provided ConnectionHandle does not match any connection\nLikely the connection was deleted before!");