From d001a5fba643510566917785c7f3926dd05b9b44 Mon Sep 17 00:00:00 2001
From: Kasper Marstal <kaspermarstal@gmail.com>
Date: Wed, 28 Oct 2015 09:45:20 +0100
Subject: [PATCH] ENH: ELASTIX-9 Only access connections via component indexes;
BUG: ELASTIX-9 ComponentExist() did not work
The ENH: We deliberately avoid using connection indexes, but instead
force the user to think in terms of components which is conceptually
simpler and results in less pointers being thrown around while
constructing the blueprint.
The BUG: Temporarily disable functionality for deleting component until
we find a god way of asserting that vertex exist before it is accessed.
---
.../Core/Blueprints/include/elxBlueprint.h | 36 +++++--
Modules/Core/Blueprints/src/elxBlueprint.cxx | 99 ++++++++++++++++---
Testing/Unit/elxBluePrintTest.cxx | 30 +++---
3 files changed, 131 insertions(+), 34 deletions(-)
diff --git a/Modules/Core/Blueprints/include/elxBlueprint.h b/Modules/Core/Blueprints/include/elxBlueprint.h
index bf31c28e..ce185437 100644
--- a/Modules/Core/Blueprints/include/elxBlueprint.h
+++ b/Modules/Core/Blueprints/include/elxBlueprint.h
@@ -33,8 +33,8 @@ public:
ParameterMapType parameterMap;
};
- typedef boost::adjacency_list< boost::vecS,
- boost::vecS,
+ typedef boost::adjacency_list< boost::listS,
+ boost::listS,
boost::directedS,
ComponentPropertyType,
ConnectionPropertyType > GraphType;
@@ -57,23 +57,45 @@ public:
ComponentIndexType AddComponent( void );
ComponentIndexType AddComponent( ParameterMapType parameterMap );
ParameterMapType GetComponent( ComponentIndexType index );
- bool SetComponent( ComponentIndexType, ParameterMapType parameterMap );
- void DeleteComponent( ComponentIndexType );
- bool ComponentExist( ComponentIndexType index );
+ void SetComponent( ComponentIndexType, ParameterMapType parameterMap );
+
+ // TODO: Let user delete component. To do this, we need a proper way of
+ // checking that a vertex exist. Otherwise a call to GetComponent() on
+ // a deleted vertex will result in segfault. It is not really a problem
+ // that there is no delete vertex feature at this point since typically
+ // the blueprint interface is used procedurally to generate a specific
+ // blueprint.
+ // void DeleteComponent( ComponentIndexType );
ComponentIteratorPairType GetComponentIterator( void ) {
- return boost::vertices(this->m_Graph);
+ return boost::vertices( this->m_Graph );
}
+ // Interface for managing connections between components in which we
+ // deliberately avoid using connection indexes, but instead force
+ // the user to think in terms of components (which is conceptually simpler)
+ bool AddConnection( ComponentIndexType upstream, ComponentIndexType downstream );
+ bool AddConnection( ComponentIndexType upstream, ComponentIndexType downstream, ParameterMapType parameterMap );
+ ParameterMapType GetConnection( ComponentIndexType upstream, ComponentIndexType downstream );
+ bool SetConnection( ComponentIndexType upstream, ComponentIndexType downstream, ParameterMapType parameterMap );
+ bool DeleteConnection( ComponentIndexType upstream, ComponentIndexType downstream );
+ bool ConnectionExists( ComponentIndexType upstream, ComponentIndexType downstream );
+
+ // Returns iterator for all connections in the graph
+ ConnectionIteratorPairType GetConnectionIterator( void ) {
+ return boost::edges(this->m_Graph);
+ }
// Returns the outgoing connections from a component in the graph,
// i.e. all components that reads data from given component
- OutputIteratorPairType GetOuptutIterator( const ComponentIndexType index ) {
+ OutputIteratorPairType GetOutputIterator( const ComponentIndexType index ) {
return boost::out_edges(index, this->m_Graph);
}
private:
+ ConnectionIndexType GetConnectionIndex( ComponentIndexType upsteam, ComponentIndexType downstream );
+
GraphType m_Graph;
};
diff --git a/Modules/Core/Blueprints/src/elxBlueprint.cxx b/Modules/Core/Blueprints/src/elxBlueprint.cxx
index 451f0b73..278725aa 100644
--- a/Modules/Core/Blueprints/src/elxBlueprint.cxx
+++ b/Modules/Core/Blueprints/src/elxBlueprint.cxx
@@ -38,46 +38,113 @@ Blueprint
{
this->Modified();
- if( this->ComponentExist( index ) ) {
- return this->m_Graph[index].parameterMap;
- } else {
- itkExceptionMacro( "Blueprint does not contain component with index " << index );
+ return this->m_Graph[ index ].parameterMap;
+}
+
+void
+Blueprint
+::SetComponent( ComponentIndexType index, ParameterMapType parameterMap )
+{
+ this->Modified();
+ this->m_Graph[ index ].parameterMap = parameterMap;
+}
+
+// TODO: See explanation in elxBlueprint.h
+// void
+// Blueprint
+// ::DeleteComponent( const ComponentIndexType index )
+// {
+// this->Modified();
+//
+// clear_vertex( index, this->m_Graph );
+// remove_vertex( index, this->m_Graph );
+// }
+
+bool
+Blueprint
+::AddConnection( ComponentIndexType upstream, ComponentIndexType downstream )
+{
+ this->Modified();
+
+ if( this->ConnectionExists( upstream, downstream) ) {
+ return false;
}
+
+ // Adds directed connection from upstream component to downstream component
+ return boost::add_edge( upstream, downstream, this->m_Graph ).second;
}
bool
Blueprint
-::SetComponent( ComponentIndexType index, ParameterMapType parameterMap )
+::AddConnection( ComponentIndexType upstream, ComponentIndexType downstream, ParameterMapType parameterMap )
+{
+ this->Modified();
+
+ // If the connection does not exist, add the parameter map, otherwise don't do anything
+ // because previous settings will be overwritten. If the user do want to overwrite
+ // current settings, she should use SetConnection() instead where the intent is explicit.
+ if( this->ConnectionExists( upstream, downstream) ) {
+ return false;
+ }
+
+ this->m_Graph[ this->GetConnectionIndex( upstream, downstream ) ].parameterMap = parameterMap;
+ return true;
+}
+
+Blueprint::ParameterMapType
+Blueprint
+::GetConnection( ComponentIndexType upstream, ComponentIndexType downstream )
{
this->Modified();
- if( this->ComponentExist( index ) )
- {
- this->m_Graph[index].parameterMap = parameterMap;
+ return this->m_Graph[ this->GetConnectionIndex( upstream, downstream ) ].parameterMap;
+}
+
+bool
+Blueprint
+::SetConnection( ComponentIndexType upstream, ComponentIndexType downstream, ParameterMapType parameterMap )
+{
+ this->Modified();
+
+ if( !this->ConnectionExists( upstream, downstream ) ) {
+ return this->AddConnection( upstream, downstream, parameterMap );
} else {
- itkExceptionMacro( "Blueprint does not contain component with index " << index )
+ this->m_Graph[ this->GetConnectionIndex( upstream, downstream ) ].parameterMap = parameterMap;
+ return true;
}
}
-void
+bool
Blueprint
-::DeleteComponent( const ComponentIndexType index )
+::DeleteConnection( ComponentIndexType upstream, ComponentIndexType downstream )
{
this->Modified();
- if( this->ComponentExist( index ) ) {
- clear_vertex( index, this->m_Graph );
- remove_vertex( index, this->m_Graph );
+ if( this->ConnectionExists( upstream, downstream ) ) {
+ this->m_Graph.remove_edge( this->GetConnectionIndex( upstream, downstream ) );
}
+
+ return !this->ConnectionExists( upstream, downstream );
}
bool
Blueprint
-::ComponentExist( ComponentIndexType index )
+::ConnectionExists( ComponentIndexType upstream, ComponentIndexType downstream )
{
- return boost::vertex( index, this->m_Graph ) != boost::graph_traits< GraphType >::null_vertex();
+ return boost::edge( upstream, downstream, this->m_Graph).second;
}
+Blueprint::ConnectionIndexType
+Blueprint
+::GetConnectionIndex( ComponentIndexType upstream, ComponentIndexType downstream )
+{
+ // This function is part of the internal API and should fail hard if we use it incorrectly
+ if( !this->ConnectionExists( upstream, downstream ) ) {
+ itkExceptionMacro( "Blueprint does not contain connection from component " << upstream << " to " << downstream );
+ }
+
+ return boost::edge( upstream, downstream, this->m_Graph).first;
+}
// void
// Blueprint
diff --git a/Testing/Unit/elxBluePrintTest.cxx b/Testing/Unit/elxBluePrintTest.cxx
index a6b3e810..f2c39fdf 100644
--- a/Testing/Unit/elxBluePrintTest.cxx
+++ b/Testing/Unit/elxBluePrintTest.cxx
@@ -19,44 +19,52 @@ public:
ParameterMapType parameterMap;
};
-TEST_F( BlueprintTest, Add )
+TEST_F( BlueprintTest, AddComponent )
{
BlueprintPointerType blueprint;
EXPECT_NO_THROW( blueprint = Blueprint::New() );
ComponentIndexType index0;
EXPECT_NO_THROW( index0 = blueprint->AddComponent() );
- EXPECT_EQ( Blueprint::ComponentIndexType(0), index0 );
ComponentIndexType index1;
EXPECT_NO_THROW( index1 = blueprint->AddComponent( parameterMap ) );
- EXPECT_EQ( Blueprint::ComponentIndexType(1), index1 );
-
- Blueprint::ComponentIteratorPairType componentIterator = blueprint->GetComponentIterator();
}
-TEST_F( BlueprintTest, Get )
+TEST_F( BlueprintTest, GetComponent )
{
BlueprintPointerType blueprint = Blueprint::New();
ComponentIndexType index = blueprint->AddComponent( parameterMap );
- EXPECT_EQ( Blueprint::ComponentIndexType(0), index );
ParameterMapType parameterMapTest;
EXPECT_NO_THROW( parameterMapTest = blueprint->GetComponent( index ) );
EXPECT_EQ( parameterMap["ComponentName"], parameterMapTest["ComponentName"] );
}
-TEST_F( BlueprintTest, Delete )
+TEST_F( BlueprintTest, SetComponent )
{
BlueprintPointerType blueprint = Blueprint::New();
ComponentIndexType index = blueprint->AddComponent( parameterMap );
ParameterMapType parameterMapTest;
+ EXPECT_NO_THROW( blueprint->SetComponent( index, parameterMap ) );
EXPECT_NO_THROW( parameterMapTest = blueprint->GetComponent( index ) );
EXPECT_EQ( parameterMap["ComponentName"], parameterMapTest["ComponentName"] );
-
- EXPECT_NO_THROW( blueprint->DeleteComponent( index ) );
- EXPECT_ANY_THROW( parameterMapTest = blueprint->GetComponent( index ) );
}
+// TODO: The final line segfaults since at this point GetComponent has no way
+// of checking that the index actually exist. See explanation in elxBlueprint.h
+// TEST_F( BlueprintTest, DeleteComponent )
+// {
+// BlueprintPointerType blueprint = Blueprint::New();
+// ComponentIndexType index = blueprint->AddComponent( parameterMap );
+//
+// ParameterMapType parameterMapTest;
+// EXPECT_NO_THROW( parameterMapTest = blueprint->GetComponent( index ) );
+// EXPECT_EQ( parameterMap["ComponentName"], parameterMapTest["ComponentName"] );
+//
+// EXPECT_NO_THROW( blueprint->DeleteComponent( index ) );
+// EXPECT_ANY_THROW( parameterMapTest = blueprint->GetComponent( index ) );
+// }
+
} // namespace elx
--
GitLab