From 371e23985996e3e548adfa81ec0faf96c172dcbf Mon Sep 17 00:00:00 2001 From: Asher Glick Date: Fri, 29 Sep 2023 23:59:39 -0500 Subject: [PATCH 1/5] Adding string hierarchy helper functions, and fixing pointer and reference styling rules --- xml_converter/.clang-format | 3 +- xml_converter/src/string_hierarchy.cpp | 52 ++++++++++++++++++++++++++ xml_converter/src/string_hierarchy.hpp | 9 +++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/xml_converter/.clang-format b/xml_converter/.clang-format index 71dc1ff2..bf384993 100644 --- a/xml_converter/.clang-format +++ b/xml_converter/.clang-format @@ -57,7 +57,7 @@ ConstructorInitializerIndentWidth: 4 ContinuationIndentWidth: 4 Cpp11BracedListStyle: true DeriveLineEnding: true -DerivePointerAlignment: true +DerivePointerAlignment: false DisableFormat: false ExperimentalAutoDetectBinPacking: false FixNamespaceComments: true @@ -106,6 +106,7 @@ PenaltyBreakTemplateDeclaration: 10 PenaltyExcessCharacter: 1 # 1000000 PenaltyReturnTypeOnItsOwnLine: 200 PointerAlignment: Left +ReferenceAlignment: Right RawStringFormats: - Language: Cpp Delimiters: diff --git a/xml_converter/src/string_hierarchy.cpp b/xml_converter/src/string_hierarchy.cpp index 0c907c39..03371f00 100644 --- a/xml_converter/src/string_hierarchy.cpp +++ b/xml_converter/src/string_hierarchy.cpp @@ -2,6 +2,24 @@ #include +//////////////////////////////////////////////////////////////////////////////// +// in_hierarchy +// +// Returns if a particular node exists at the top level of the hirearchy. +//////////////////////////////////////////////////////////////////////////////// +bool StringHierarchy::in_hierarchy( + const std::string &node) const { + if (this->all_children_included) { + return true; + } + + auto iterator = this->children.find(node); + if (iterator == this->children.end()) { + return false; + } + return true; +} + //////////////////////////////////////////////////////////////////////////////// // in_hierarchy // @@ -39,6 +57,40 @@ bool StringHierarchy::_in_hierarchy( return iterator->second._in_hierarchy(path, continue_index + 1); } +//////////////////////////////////////////////////////////////////////////////// +// sub_hierarchy +// +// A helper function to grab a sub hierarchy one level down from the top. +//////////////////////////////////////////////////////////////////////////////// +const StringHierarchy* StringHierarchy::sub_hierarchy( + const std::string &node) const { + if (this->all_children_included) { + return this; + } + + auto iterator = this->children.find(node); + if (iterator == this->children.end()) { + return nullptr; + } + return &(iterator->second); +} + +//////////////////////////////////////////////////////////////////////////////// +// sub_hierarchy +// +// Get a subtree of the StringHierarchy in order to avoid needing to query the +// entire hierarchy in the case where the use case is traversing down a tree +// anyways and does not want to keep track of the parent's values. +//////////////////////////////////////////////////////////////////////////////// +const StringHierarchy* StringHierarchy::sub_hierarchy( + const std::vector &path) const { + const StringHierarchy* sub_hierarchy = this; + for (size_t i = 0; i < path.size(); i++) { + sub_hierarchy = this->sub_hierarchy(path[i]); + } + return sub_hierarchy; +} + //////////////////////////////////////////////////////////////////////////////// // add_path // diff --git a/xml_converter/src/string_hierarchy.hpp b/xml_converter/src/string_hierarchy.hpp index 9a5587db..47218a97 100644 --- a/xml_converter/src/string_hierarchy.hpp +++ b/xml_converter/src/string_hierarchy.hpp @@ -7,6 +7,15 @@ class StringHierarchy { public: bool in_hierarchy( + const std::string &node) const; + + bool in_hierarchy( + const std::vector &path) const; + + const StringHierarchy* sub_hierarchy( + const std::string &node) const; + + const StringHierarchy* sub_hierarchy( const std::vector &path) const; void add_path( From 3e38ff4750733eea90043104ac3eff0bfb57c157 Mon Sep 17 00:00:00 2001 From: Asher Glick Date: Sat, 30 Sep 2023 00:49:11 -0500 Subject: [PATCH 2/5] adding intializer list overloads and unit tests --- xml_converter/src/string_hierarchy.cpp | 37 ++++++++++++++- xml_converter/src/string_hierarchy.hpp | 6 +++ xml_converter/tests/test_string_hierarchy.cpp | 45 +++++++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/xml_converter/src/string_hierarchy.cpp b/xml_converter/src/string_hierarchy.cpp index 03371f00..c7583d83 100644 --- a/xml_converter/src/string_hierarchy.cpp +++ b/xml_converter/src/string_hierarchy.cpp @@ -20,6 +20,8 @@ bool StringHierarchy::in_hierarchy( return true; } + + //////////////////////////////////////////////////////////////////////////////// // in_hierarchy // @@ -30,6 +32,20 @@ bool StringHierarchy::in_hierarchy( return this->_in_hierarchy(path, 0); } +//////////////////////////////////////////////////////////////////////////////// +// in_hierarchy +// +// An explicit version of in_hierarchy that takes an initalizer list to prevent +// ambiguity between the vector and string overloads of the function. +//////////////////////////////////////////////////////////////////////////////// +bool StringHierarchy::in_hierarchy( + const std::initializer_list input +) const { + std::vector vec; + vec.insert(vec.end(), input.begin(), input.end()); + return this->in_hierarchy(vec); +} + //////////////////////////////////////////////////////////////////////////////// // _in_hirearchy // @@ -75,6 +91,21 @@ const StringHierarchy* StringHierarchy::sub_hierarchy( return &(iterator->second); } + +//////////////////////////////////////////////////////////////////////////////// +// sub_hierarchy +// +// An explicit version of sub_hierarchy that takes an initalizer list to +// prevent ambiguity between the vector and string overloads of the function. +//////////////////////////////////////////////////////////////////////////////// +const StringHierarchy* StringHierarchy::sub_hierarchy( + const std::initializer_list input) const { + std::vector vec; + vec.insert(vec.end(), input.begin(), input.end()); + return this->sub_hierarchy(vec); +} + + //////////////////////////////////////////////////////////////////////////////// // sub_hierarchy // @@ -86,7 +117,11 @@ const StringHierarchy* StringHierarchy::sub_hierarchy( const std::vector &path) const { const StringHierarchy* sub_hierarchy = this; for (size_t i = 0; i < path.size(); i++) { - sub_hierarchy = this->sub_hierarchy(path[i]); + sub_hierarchy = sub_hierarchy->sub_hierarchy(path[i]); + // Escape before segfaulting. + if (sub_hierarchy == nullptr) { + return nullptr; + } } return sub_hierarchy; } diff --git a/xml_converter/src/string_hierarchy.hpp b/xml_converter/src/string_hierarchy.hpp index 47218a97..94e2fc7c 100644 --- a/xml_converter/src/string_hierarchy.hpp +++ b/xml_converter/src/string_hierarchy.hpp @@ -12,9 +12,15 @@ class StringHierarchy { bool in_hierarchy( const std::vector &path) const; + bool in_hierarchy( + const std::initializer_list) const; + const StringHierarchy* sub_hierarchy( const std::string &node) const; + const StringHierarchy* sub_hierarchy( + const std::initializer_list input) const; + const StringHierarchy* sub_hierarchy( const std::vector &path) const; diff --git a/xml_converter/tests/test_string_hierarchy.cpp b/xml_converter/tests/test_string_hierarchy.cpp index 7eed8a30..4e2d84c8 100644 --- a/xml_converter/tests/test_string_hierarchy.cpp +++ b/xml_converter/tests/test_string_hierarchy.cpp @@ -18,6 +18,11 @@ TEST_F(StringHierarchyTest, BasicPath) { EXPECT_TRUE(string_hierarchy.in_hierarchy({"root", "child1", "child2"})); } +TEST_F(StringHierarchyTest, BasicPathSingle) { + string_hierarchy.add_path({"root", "child1", "child2"}, false); + EXPECT_TRUE(string_hierarchy.in_hierarchy("root")); +} + TEST_F(StringHierarchyTest, ParentPath) { string_hierarchy.add_path({"root", "child1", "child2"}, false); EXPECT_TRUE(string_hierarchy.in_hierarchy({"root", "child1"})); @@ -34,6 +39,11 @@ TEST_F(StringHierarchyTest, InvalidRoot) { EXPECT_FALSE(string_hierarchy.in_hierarchy({"badroot"})); } +TEST_F(StringHierarchyTest, InvalidRootSingle) { + string_hierarchy.add_path({"root", "child1", "child2"}, false); + EXPECT_FALSE(string_hierarchy.in_hierarchy("badroot")); +} + TEST_F(StringHierarchyTest, NonExistantDepthNode) { string_hierarchy.add_path({"root", "child1", "child2"}, false); EXPECT_FALSE(string_hierarchy.in_hierarchy({"root", "child1", "child2", "child3"})); @@ -67,6 +77,41 @@ TEST_F(StringHierarchyTest, AllowAll) { EXPECT_TRUE(string_hierarchy.in_hierarchy({"literally", "anything"})); } +TEST_F(StringHierarchyTest, AllowAllSingle) { + string_hierarchy.add_path({}, true); + EXPECT_TRUE(string_hierarchy.in_hierarchy("everything")); +} + +TEST_F(StringHierarchyTest, InvalidHierarchy) { + string_hierarchy.add_path({"root", "child1", "child2"}, false); + const StringHierarchy* sub_hierarchy = string_hierarchy.sub_hierarchy({"invalid"}); + EXPECT_EQ(sub_hierarchy, nullptr); +} + +TEST_F(StringHierarchyTest, SubHierarchy) { + string_hierarchy.add_path({"root", "child1", "child2"}, false); + const StringHierarchy* sub_hierarchy = string_hierarchy.sub_hierarchy({"root", "child1"}); + EXPECT_NE(sub_hierarchy, nullptr); + EXPECT_FALSE(sub_hierarchy->in_hierarchy("root")); + EXPECT_FALSE(sub_hierarchy->in_hierarchy("child1")); + EXPECT_TRUE(sub_hierarchy->in_hierarchy("child2")); +} + +TEST_F(StringHierarchyTest, InvalidDeepHierarchy) { + string_hierarchy.add_path({"root", "child1", "child2"}, false); + const StringHierarchy* sub_hierarchy = string_hierarchy.sub_hierarchy({"invalid", "nonexistant"}); + EXPECT_EQ(sub_hierarchy, nullptr); +} + +TEST_F(StringHierarchyTest, SubHierarchySingle) { + string_hierarchy.add_path({"root", "child1", "child2"}, false); + const StringHierarchy* sub_hierarchy = string_hierarchy.sub_hierarchy("root"); + EXPECT_NE(sub_hierarchy, nullptr); + EXPECT_FALSE(sub_hierarchy->in_hierarchy("root")); + EXPECT_TRUE(sub_hierarchy->in_hierarchy("child1")); + EXPECT_TRUE(sub_hierarchy->in_hierarchy({"child1", "child2"})); +} + int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); From fba68942192fcf52bc90b76f352d82d7cb1fa72d Mon Sep 17 00:00:00 2001 From: Asher Glick Date: Sat, 30 Sep 2023 00:54:24 -0500 Subject: [PATCH 3/5] reverting some of the clang format change becuase it seems to have a wider impact --- xml_converter/.clang-format | 2 +- xml_converter/src/string_hierarchy.cpp | 15 +++++---------- xml_converter/src/string_hierarchy.hpp | 6 +++--- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/xml_converter/.clang-format b/xml_converter/.clang-format index bf384993..76cbc3dc 100644 --- a/xml_converter/.clang-format +++ b/xml_converter/.clang-format @@ -57,7 +57,7 @@ ConstructorInitializerIndentWidth: 4 ContinuationIndentWidth: 4 Cpp11BracedListStyle: true DeriveLineEnding: true -DerivePointerAlignment: false +DerivePointerAlignment: true DisableFormat: false ExperimentalAutoDetectBinPacking: false FixNamespaceComments: true diff --git a/xml_converter/src/string_hierarchy.cpp b/xml_converter/src/string_hierarchy.cpp index c7583d83..62b3c35b 100644 --- a/xml_converter/src/string_hierarchy.cpp +++ b/xml_converter/src/string_hierarchy.cpp @@ -20,8 +20,6 @@ bool StringHierarchy::in_hierarchy( return true; } - - //////////////////////////////////////////////////////////////////////////////// // in_hierarchy // @@ -39,8 +37,7 @@ bool StringHierarchy::in_hierarchy( // ambiguity between the vector and string overloads of the function. //////////////////////////////////////////////////////////////////////////////// bool StringHierarchy::in_hierarchy( - const std::initializer_list input -) const { + const std::initializer_list input) const { std::vector vec; vec.insert(vec.end(), input.begin(), input.end()); return this->in_hierarchy(vec); @@ -78,7 +75,7 @@ bool StringHierarchy::_in_hierarchy( // // A helper function to grab a sub hierarchy one level down from the top. //////////////////////////////////////////////////////////////////////////////// -const StringHierarchy* StringHierarchy::sub_hierarchy( +const StringHierarchy *StringHierarchy::sub_hierarchy( const std::string &node) const { if (this->all_children_included) { return this; @@ -91,21 +88,19 @@ const StringHierarchy* StringHierarchy::sub_hierarchy( return &(iterator->second); } - //////////////////////////////////////////////////////////////////////////////// // sub_hierarchy // // An explicit version of sub_hierarchy that takes an initalizer list to // prevent ambiguity between the vector and string overloads of the function. //////////////////////////////////////////////////////////////////////////////// -const StringHierarchy* StringHierarchy::sub_hierarchy( +const StringHierarchy *StringHierarchy::sub_hierarchy( const std::initializer_list input) const { std::vector vec; vec.insert(vec.end(), input.begin(), input.end()); return this->sub_hierarchy(vec); } - //////////////////////////////////////////////////////////////////////////////// // sub_hierarchy // @@ -113,9 +108,9 @@ const StringHierarchy* StringHierarchy::sub_hierarchy( // entire hierarchy in the case where the use case is traversing down a tree // anyways and does not want to keep track of the parent's values. //////////////////////////////////////////////////////////////////////////////// -const StringHierarchy* StringHierarchy::sub_hierarchy( +const StringHierarchy *StringHierarchy::sub_hierarchy( const std::vector &path) const { - const StringHierarchy* sub_hierarchy = this; + const StringHierarchy *sub_hierarchy = this; for (size_t i = 0; i < path.size(); i++) { sub_hierarchy = sub_hierarchy->sub_hierarchy(path[i]); // Escape before segfaulting. diff --git a/xml_converter/src/string_hierarchy.hpp b/xml_converter/src/string_hierarchy.hpp index 94e2fc7c..3b323b11 100644 --- a/xml_converter/src/string_hierarchy.hpp +++ b/xml_converter/src/string_hierarchy.hpp @@ -15,13 +15,13 @@ class StringHierarchy { bool in_hierarchy( const std::initializer_list) const; - const StringHierarchy* sub_hierarchy( + const StringHierarchy *sub_hierarchy( const std::string &node) const; - const StringHierarchy* sub_hierarchy( + const StringHierarchy *sub_hierarchy( const std::initializer_list input) const; - const StringHierarchy* sub_hierarchy( + const StringHierarchy *sub_hierarchy( const std::vector &path) const; void add_path( From b68be589686b8fdd6e76ef083d36914a76309626 Mon Sep 17 00:00:00 2001 From: Asher Glick Date: Sat, 30 Sep 2023 01:02:38 -0500 Subject: [PATCH 4/5] fully reverting the clang format change --- xml_converter/.clang-format | 1 - 1 file changed, 1 deletion(-) diff --git a/xml_converter/.clang-format b/xml_converter/.clang-format index 76cbc3dc..71dc1ff2 100644 --- a/xml_converter/.clang-format +++ b/xml_converter/.clang-format @@ -106,7 +106,6 @@ PenaltyBreakTemplateDeclaration: 10 PenaltyExcessCharacter: 1 # 1000000 PenaltyReturnTypeOnItsOwnLine: 200 PointerAlignment: Left -ReferenceAlignment: Right RawStringFormats: - Language: Cpp Delimiters: From aede4e48461bfd5a7425e3c5d5723d7f2dc4f1f2 Mon Sep 17 00:00:00 2001 From: Asher Glick Date: Wed, 4 Oct 2023 10:36:45 -0500 Subject: [PATCH 5/5] passing the initalizer list by reference --- xml_converter/src/string_hierarchy.cpp | 4 ++-- xml_converter/src/string_hierarchy.hpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/xml_converter/src/string_hierarchy.cpp b/xml_converter/src/string_hierarchy.cpp index 62b3c35b..f14fe51b 100644 --- a/xml_converter/src/string_hierarchy.cpp +++ b/xml_converter/src/string_hierarchy.cpp @@ -37,7 +37,7 @@ bool StringHierarchy::in_hierarchy( // ambiguity between the vector and string overloads of the function. //////////////////////////////////////////////////////////////////////////////// bool StringHierarchy::in_hierarchy( - const std::initializer_list input) const { + const std::initializer_list &input) const { std::vector vec; vec.insert(vec.end(), input.begin(), input.end()); return this->in_hierarchy(vec); @@ -95,7 +95,7 @@ const StringHierarchy *StringHierarchy::sub_hierarchy( // prevent ambiguity between the vector and string overloads of the function. //////////////////////////////////////////////////////////////////////////////// const StringHierarchy *StringHierarchy::sub_hierarchy( - const std::initializer_list input) const { + const std::initializer_list &input) const { std::vector vec; vec.insert(vec.end(), input.begin(), input.end()); return this->sub_hierarchy(vec); diff --git a/xml_converter/src/string_hierarchy.hpp b/xml_converter/src/string_hierarchy.hpp index 3b323b11..594300ba 100644 --- a/xml_converter/src/string_hierarchy.hpp +++ b/xml_converter/src/string_hierarchy.hpp @@ -13,13 +13,13 @@ class StringHierarchy { const std::vector &path) const; bool in_hierarchy( - const std::initializer_list) const; + const std::initializer_list &input) const; const StringHierarchy *sub_hierarchy( const std::string &node) const; const StringHierarchy *sub_hierarchy( - const std::initializer_list input) const; + const std::initializer_list &input) const; const StringHierarchy *sub_hierarchy( const std::vector &path) const;