From d0614266a818739a75eb6111b02061f35f30cee9 Mon Sep 17 00:00:00 2001
From: Matthias Killat <matthias.killat@apex.ai>
Date: Tue, 20 Sep 2022 10:19:43 +0200
Subject: [PATCH] iox-#1640 Document PolymorphicHandler

Signed-off-by: Matthias Killat <matthias.killat@apex.ai>
---
 .../design_pattern/polymorphic_handler.hpp    | 43 ++++++++++++++++---
 .../moduletests/test_polymorphic_handler.cpp  |  6 +--
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/design_pattern/polymorphic_handler.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/design_pattern/polymorphic_handler.hpp
index a45d50edb34..2fe23de1916 100644
--- a/iceoryx_hoofs/include/iceoryx_hoofs/design_pattern/polymorphic_handler.hpp
+++ b/iceoryx_hoofs/include/iceoryx_hoofs/design_pattern/polymorphic_handler.hpp
@@ -8,42 +8,62 @@
 // TODO: change namespace? but DesignPattern is a bad namespace name ...
 namespace iox
 {
+namespace design
+{
+// TODO: namespace of this concept? should not depend on templated class (e.g. PolymorphicHandler)
 
-struct Activatable
+/// @brief Implements the Activatable concept to be used in the PolymorphicHandler
+///        The concept implements a binary switch. By default is switched on (active).
+class Activatable
 {
+  public:
     Activatable() = default;
 
+    /// @brief Switch on.
     void activate()
     {
         m_active = true;
     }
 
+    /// @brief Switch off.
     void deactivate()
     {
         m_active = false;
     }
 
+    /// @brief Query switch state.
+    /// @return true if active (on), false otherwise (off).
     bool isActive() const
     {
         return m_active;
     }
 
   private:
-    bool m_active = true;
+    bool m_active{true};
 };
 
-// we lose generality now wrt. the interface (require activate, deactivate etc. in the interface)
+/// @brief Implements a singleton handler that has a default instance and can be changed
+///        to another instance at runtime. All instances have to derive from the same interface.
+///        The singleton handler owns the default instance but all other instances are created externally.
+/// @tparam Interface The interface of the handler instances. Must inherit from Activatable.
+/// @tparam Default The type of the default instance. Must be equal to or derive from Interface.
+///
+/// @note In the special case where Default equals Interface, no polymorphism is required.
+///       It is then possible to e.g. switch between multiple instances of Default type.
+/// @note The lifetime of external non-default instances must exceed the lifetime of the PolymorphicHandler.
+/// @note The PolymorphicHandler is guaranteed to provide a valid handler during the whole program lifetime (static).
+///       It is hence not advisable to have other static variables depend on the PolymorphicHandler.
+///       It must be ensured that the are destroyed before the PolymorphicHandler.
 template <typename Interface, typename Default>
 class PolymorphicHandler
 {
     static_assert(std::is_base_of<Interface, Default>::value, "Default must inherit from Interface");
 
     // actually it suffices to provide the methods activate, deactivate, isActive
+    // but they need to behave correctly and inheritance enforces this
     static_assert(std::is_base_of<Activatable, Interface>::value, "Interface must inherit from Activatable");
 
   public:
-    using Self = PolymorphicHandler<Interface, Default>;
-
     // on first call (in a thread):
     // 1. localHandler is initialized
     //    - getCurrent is called
@@ -66,8 +86,11 @@ class PolymorphicHandler
     // under lock, we update the local handler to the new one (note that it cannot change
     // while this happens as we hold the lock)
 
+    /// @brief get the current singleton instance
+    /// @return the current instance
     static Interface& get()
     {
+        // NOLINTNEXTLINE
         thread_local Interface* localHandler = getCurrent(); // initialized once per thread on first call
 
         if (!localHandler->isActive())
@@ -78,6 +101,9 @@ class PolymorphicHandler
         return *localHandler;
     }
 
+    /// @brief set the current singleton instance
+    /// @param handler the handler instance to be set
+    /// @return pointer to the previous instance
     static Interface* set(Interface& handler)
     {
         auto& ins = instance();
@@ -100,11 +126,15 @@ class PolymorphicHandler
         return prev;
     }
 
+    /// @brief reset the current singleton instance to the default instance
+    /// @return pointer to the previous instance
     static Interface* reset()
     {
         return set(getDefault());
     }
 
+    /// @brief finalizes the instance, afterwards no further instance can be set
+    ///        during program lifetime
     static void finalize()
     {
         auto& ins = instance();
@@ -146,4 +176,5 @@ class PolymorphicHandler
     }
 };
 
-} // namespace eh
\ No newline at end of file
+} // namespace design
+} // namespace iox
\ No newline at end of file
diff --git a/iceoryx_hoofs/test/moduletests/test_polymorphic_handler.cpp b/iceoryx_hoofs/test/moduletests/test_polymorphic_handler.cpp
index b99394d61cf..fbb508b30f9 100644
--- a/iceoryx_hoofs/test/moduletests/test_polymorphic_handler.cpp
+++ b/iceoryx_hoofs/test/moduletests/test_polymorphic_handler.cpp
@@ -23,7 +23,7 @@ namespace
 {
 using namespace ::testing;
 
-struct Interface : public iox::Activatable
+struct Interface : public iox::design::Activatable
 {
     virtual ~Interface() = default;
 
@@ -61,7 +61,7 @@ Alternate& alternateHandler()
     return h;
 };
 
-using Handler = iox::PolymorphicHandler<Interface, Default>;
+using Handler = iox::design::PolymorphicHandler<Interface, Default>;
 
 class PolymorphicHandler_test : public Test
 {
@@ -177,7 +177,7 @@ class Activatable_test : public Test
         }
     }
 
-    iox::Activatable sut;
+    iox::design::Activatable sut;
 };
 
 TEST_F(Activatable_test, isActiveAfterConstruction)