Skip to content

Commit 4af113d

Browse files
lemireplemirep
committed
destruction_helpers.h: remove using namespace KDBindings
Because headers should never have a using namespace declaration. Also fix typos, add unit tests and minimal documentation. Change-Id: Ic524c53417da7273c15c1c9eaa5f8ef8758340af
1 parent ef38222 commit 4af113d

File tree

4 files changed

+161
-11
lines changed

4 files changed

+161
-11
lines changed

‎src/KDFoundation/destruction_helpers.h‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616

1717
#include <map>
1818

19-
using namespace KDBindings;
20-
2119
// TODO: Maybe belongs to ecs?
2220

2321
namespace KDFoundation {
@@ -26,22 +24,22 @@ template<typename ContainerType, typename DependencyType>
2624
class DestructionHelperManager
2725
{
2826
public:
29-
ConnectionHandle contToDepConnection(std::pair<ContainerType *, DependencyType *> contDepPair)
27+
KDBindings::ConnectionHandle contToDepConnection(std::pair<ContainerType *, DependencyType *> contDepPair)
3028
{
3129
return m_contToDepMap[contDepPair];
3230
}
3331

34-
ConnectionHandle depToContConnection(std::pair<DependencyType *, ContainerType *> depContPair)
32+
KDBindings::ConnectionHandle depToContConnection(std::pair<DependencyType *, ContainerType *> depContPair)
3533
{
3634
return m_depToContMap[depContPair];
3735
}
3836

39-
void addContToDepConnection(std::pair<ContainerType *, DependencyType *> contDepPair, ConnectionHandle handle)
37+
void addContToDepConnection(std::pair<ContainerType *, DependencyType *> contDepPair, KDBindings::ConnectionHandle handle)
4038
{
4139
m_contToDepMap[contDepPair] = handle;
4240
}
4341

44-
void addDepToContConnection(std::pair<DependencyType *, ContainerType *> depContPair, ConnectionHandle handle)
42+
void addDepToContConnection(std::pair<DependencyType *, ContainerType *> depContPair, KDBindings::ConnectionHandle handle)
4543
{
4644
m_depToContMap[depContPair] = handle;
4745
}
@@ -62,7 +60,7 @@ class DestructionHelperManager
6260
}
6361
}
6462

65-
void clearDepToContConnctions()
63+
void clearDepToContConnections()
6664
{
6765
for (const auto &depConn : m_depToContMap) {
6866
depConn.first.first->destroyed.disconnect(depConn.second);
@@ -71,12 +69,14 @@ class DestructionHelperManager
7169
}
7270

7371
private:
74-
std::map<std::pair<ContainerType *, DependencyType *>, ConnectionHandle> m_contToDepMap;
75-
std::map<std::pair<DependencyType *, ContainerType *>, ConnectionHandle> m_depToContMap;
72+
std::map<std::pair<ContainerType *, DependencyType *>, KDBindings::ConnectionHandle> m_contToDepMap;
73+
std::map<std::pair<DependencyType *, ContainerType *>, KDBindings::ConnectionHandle> m_depToContMap;
7674
};
7775

76+
// Watch object held by property of type KDBinding::Property<DependencyType *> and resets property value if node is destroyed
77+
// Optional property owner can be passed in
7878
template<typename DependencyType, typename ContainerType>
79-
void registerDestructionHelper(Property<DependencyType *> *property, ConnectionHandle *connectionHandle, ContainerType *owner)
79+
void registerDestructionHelper(KDBindings::Property<DependencyType *> *property, KDBindings::ConnectionHandle *connectionHandle, ContainerType *owner)
8080
{
8181
std::ignore = property->valueAboutToChange().connect([=](DependencyType *oldVal, DependencyType * /* newVal */) {
8282
if (!oldVal) {
@@ -101,6 +101,7 @@ void registerDestructionHelper(Property<DependencyType *> *property, ConnectionH
101101
}
102102
}
103103

104+
// Triggers dependencyCleanupFunctor when DependencyType instance referenced by Container is destroyed
104105
template<typename ContainerType, typename DependencyType, typename OnDepenencyDestructionFunctorType>
105106
void registerDestructionHelper(ContainerType *container, DependencyType *dependency, DestructionHelperManager<ContainerType, DependencyType> *manager, OnDepenencyDestructionFunctorType dependencyCleanupFunctor)
106107
{
@@ -115,7 +116,7 @@ void registerDestructionHelper(ContainerType *container, DependencyType *depende
115116

116117
if (container) {
117118
auto contToDep = container->destroyed.connect([=] {
118-
manager->clearDepToContConnctions();
119+
manager->clearDepToContConnections();
119120
manager->delContToDepConnection({ container, dependency });
120121
});
121122

‎tests/auto/foundation/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ add_subdirectory(core_application)
4040
add_subdirectory(event)
4141
add_subdirectory(event_queue)
4242
add_subdirectory(object)
43+
add_subdirectory(destruction_helper)
4344

4445
if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
4546
add_subdirectory(linux_platform_event_loop)

‎tests/auto/foundation/destruction_helper/CMakeLists.txt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# This file is part of KDUtils.
2+
#
3+
# SPDX-FileCopyrightText: 2025 Klarälvdalens Datakonsult AB, a KDAB Group company <info@kdab.com>
4+
# Author: Paul Lemire <paul.lemire@kdab.com>
5+
#
6+
# SPDX-License-Identifier: MIT
7+
#
8+
# Contact KDAB at <info@kdab.com> for commercial licensing options.
9+
#
10+
11+
project(
12+
test-core-destruction-helper
13+
VERSION 0.1
14+
LANGUAGES CXX
15+
)
16+
17+
add_core_test(${PROJECT_NAME} tst_destruction_helper.cpp)

‎tests/auto/foundation/destruction_helper/tst_destruction_helper.cpp‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/*
2+
This file is part of KDUtils.
3+
4+
SPDX-FileCopyrightText: 2025 Klarälvdalens Datakonsult AB, a KDAB Group company <info@kdab.com>
5+
Author: Paul Lemire <paul.lemire@kdab.com>
6+
7+
SPDX-License-Identifier: MIT
8+
9+
Contact KDAB at <info@kdab.com> for commercial licensing options.
10+
*/
11+
12+
#include <KDFoundation/destruction_helpers.h>
13+
#include <KDFoundation/object.h>
14+
#include <signal_spy.h>
15+
16+
#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
17+
#include <doctest.h>
18+
19+
using namespace KDFoundation;
20+
21+
class Node : public Object
22+
{
23+
public:
24+
Node()
25+
{
26+
}
27+
28+
// NOLINTBEGIN(bugprone-exception-escape)
29+
~Node()
30+
{
31+
destroyed.emit(this);
32+
}
33+
// NOLINTEND(bugprone-exception-escape)
34+
35+
KDBindings::Signal<Node *> destroyed;
36+
KDBindings::Property<Node *> prop{ nullptr };
37+
};
38+
39+
TEST_SUITE("DesctructionHelper")
40+
{
41+
TEST_CASE("registerDestructionHelper Container & Dependency")
42+
{
43+
// GIVEN
44+
KDFoundation::DestructionHelperManager<Node, Node> destructionManager;
45+
Node container;
46+
int dependencyDestroyedNotificationCount = 0;
47+
48+
// WHEN
49+
{
50+
Node dependency;
51+
registerDestructionHelper(&container, &dependency, &destructionManager, [&]() {
52+
++dependencyDestroyedNotificationCount;
53+
});
54+
}
55+
56+
// THEN
57+
CHECK(dependencyDestroyedNotificationCount == 1);
58+
}
59+
60+
TEST_CASE("unregisterDestructionHelper Container & Dependency")
61+
{
62+
// GIVEN
63+
KDFoundation::DestructionHelperManager<Node, Node> destructionManager;
64+
Node container;
65+
int dependencyDestroyedNotificationCount = 0;
66+
67+
// WHEN
68+
{
69+
Node dependency;
70+
registerDestructionHelper(&container, &dependency, &destructionManager, [&]() {
71+
++dependencyDestroyedNotificationCount;
72+
});
73+
unregisterDestructionHelper(&container, &dependency, &destructionManager);
74+
}
75+
76+
// THEN
77+
CHECK(dependencyDestroyedNotificationCount == 0);
78+
}
79+
80+
TEST_CASE("registerDestructionHelper Property")
81+
{
82+
// GIVEN
83+
KDBindings::Property<Node *> dependencyProp{ nullptr };
84+
KDBindings::ConnectionHandle connection;
85+
86+
// WHEN
87+
{
88+
Node dependency;
89+
registerDestructionHelper(&dependencyProp, &connection, static_cast<Node *>(nullptr));
90+
dependencyProp = &dependency;
91+
92+
// THEN
93+
CHECK(dependencyProp() == &dependency);
94+
CHECK(connection.isActive());
95+
}
96+
97+
// cppcheck-suppress invalidLifetime
98+
CHECK(dependencyProp() == nullptr);
99+
CHECK(!connection.isActive());
100+
}
101+
102+
TEST_CASE("registerDestructionHelper Property & Owner")
103+
{
104+
// GIVEN
105+
KDBindings::ConnectionHandle connection;
106+
107+
// WHEN
108+
{
109+
Node container;
110+
111+
{
112+
Node dependency;
113+
114+
// WHEN
115+
registerDestructionHelper(&container.prop, &connection, &container);
116+
container.prop = &dependency;
117+
118+
// THEN -> Dependency is watched
119+
CHECK(container.prop() == &dependency);
120+
CHECK(connection.isActive());
121+
}
122+
123+
// THEN -> Property was reset since watched node was destroyed
124+
CHECK(container.prop() == nullptr);
125+
CHECK(!connection.isActive());
126+
}
127+
128+
// THEN -> Connectio resets since owner destroyed
129+
CHECK(!connection.isActive());
130+
}
131+
}

0 commit comments

Comments
 (0)