From 0b0141809be1d20248a39f16d4ed25a9cc64afea Mon Sep 17 00:00:00 2001
From: Christoph Lehmann <christoph.lehmann@ufz.de>
Date: Fri, 15 Jan 2016 19:42:19 +0100
Subject: [PATCH] [BL,T] improved move assignment
---
BaseLib/ConfigTreeNew.cpp | 54 ++++++++++++++++++--------------
BaseLib/ConfigTreeNew.h | 11 +++++--
Tests/BaseLib/TestConfigTree.cpp | 35 ++++++++++++++++++++-
3 files changed, 73 insertions(+), 27 deletions(-)
diff --git a/BaseLib/ConfigTreeNew.cpp b/BaseLib/ConfigTreeNew.cpp
index eed2bfb36cb..8373a7facfc 100644
--- a/BaseLib/ConfigTreeNew.cpp
+++ b/BaseLib/ConfigTreeNew.cpp
@@ -54,37 +54,21 @@ ConfigTreeNew(ConfigTreeNew && other)
ConfigTreeNew::~ConfigTreeNew()
{
- if (!_tree) return;
-
- for (auto const& p : *_tree)
- {
- markVisitedDecrement(p.first);
- }
-
- for (auto const& p : _visited_params)
- {
- if (p.second.count > 0) {
- warning("Key <" + p.first + "> has been read " + std::to_string(p.second.count)
- + " time(s) more than it was present in the configuration tree.");
- } else if (p.second.count < 0) {
- warning("Key <" + p.first + "> has been read " + std::to_string(-p.second.count)
- + " time(s) less than it was present in the configuration tree.");
- }
- }
+ checkFullyRead();
}
ConfigTreeNew&
ConfigTreeNew::
operator=(ConfigTreeNew&& other)
{
- _tree = other._tree;
- other._tree = nullptr;
- _path = other._path;
- _visited_params = std::move(other._visited_params);
+ checkFullyRead();
- // TODO Caution: That might be a very nontrivial operation (copying a std::function).
- _onerror = other._onerror;
- _onwarning = other._onwarning;
+ _tree = other._tree;
+ other._tree = nullptr;
+ _path = std::move(other._path);
+ _visited_params = std::move(other._visited_params);
+ _onerror = std::move(other._onerror);
+ _onwarning = std::move(other._onwarning);
return *this;
}
@@ -236,4 +220,26 @@ markVisitedDecrement(std::string const& key) const
}
}
+void
+ConfigTreeNew::checkFullyRead() const
+{
+ if (!_tree) return;
+
+ for (auto const& p : *_tree)
+ {
+ markVisitedDecrement(p.first);
+ }
+
+ for (auto const& p : _visited_params)
+ {
+ if (p.second.count > 0) {
+ warning("Key <" + p.first + "> has been read " + std::to_string(p.second.count)
+ + " time(s) more than it was present in the configuration tree.");
+ } else if (p.second.count < 0) {
+ warning("Key <" + p.first + "> has been read " + std::to_string(-p.second.count)
+ + " time(s) less than it was present in the configuration tree.");
+ }
+ }
+}
+
}
diff --git a/BaseLib/ConfigTreeNew.h b/BaseLib/ConfigTreeNew.h
index a14748733b2..50f55b8ea91 100644
--- a/BaseLib/ConfigTreeNew.h
+++ b/BaseLib/ConfigTreeNew.h
@@ -192,8 +192,6 @@ public:
//! used anymore!
ConfigTreeNew(ConfigTreeNew && other);
- ConfigTreeNew() = delete;
-
//! copying is not compatible with the semantics of this class
ConfigTreeNew& operator=(ConfigTreeNew const&) = delete;
@@ -364,6 +362,10 @@ private:
//! and the number of times it exists in the ConfigTree
void markVisitedDecrement(std::string const& key) const;
+ //! Helper method that checks if the top level of this tree has
+ //! been red entirely (and not too often).
+ void checkFullyRead() const;
+
//! returns a short string at suitable for error/warning messages
static std::string shortString(std::string const& s);
@@ -375,6 +377,11 @@ private:
//! A map key -> (count, type) keeping track which parameters have been read how often
//! and which datatype they have.
+ //!
+ //! This member will be written to when reading from the config tree.
+ //! Therefore it has to be mutable in order to be able to read from
+ //! constant instances, e.g., those passed as constant references to
+ //! temporaries.
mutable std::map<std::string, CountType> _visited_params;
Callback _onerror;
diff --git a/Tests/BaseLib/TestConfigTree.cpp b/Tests/BaseLib/TestConfigTree.cpp
index 250ac3e15dc..7b5bb62362e 100644
--- a/Tests/BaseLib/TestConfigTree.cpp
+++ b/Tests/BaseLib/TestConfigTree.cpp
@@ -400,7 +400,7 @@ TEST(BaseLibConfigTree, ConfigTreeStringLiterals)
}
// String literals are somewhat special for template classes
-TEST(BaseLibConfigTree, ConfigTreeMove)
+TEST(BaseLibConfigTree, ConfigTreeMoveConstruct)
{
const char xml[] =
"<s>test</s>"
@@ -428,3 +428,36 @@ TEST(BaseLibConfigTree, ConfigTreeMove)
DO_EXPECT(cbs, false, false);
}
+// String literals are somewhat special for template classes
+TEST(BaseLibConfigTree, ConfigTreeMoveAssign)
+{
+ const char xml[] =
+ "<s>test</s>"
+ "<t>Test</t>";
+
+ boost::property_tree::ptree ptree;
+ std::istringstream xml_str(xml);
+ read_xml(xml_str, ptree);
+
+ Callbacks cbs;
+ {
+ BaseLib::ConfigTreeNew conf(ptree, cbs.get_error_cb(), cbs.get_warning_cb());
+
+ EXPECT_EQ("test", conf.getConfParam<std::string>("s", "XX"));
+ DO_EXPECT(cbs, false, false);
+
+ BaseLib::ConfigTreeNew conf2(ptree, cbs.get_error_cb(), cbs.get_warning_cb());
+ conf2 = std::move(conf);
+ // Expect warning because config tree has not been traversed
+ // entirely before.
+ DO_EXPECT(cbs, false, true);
+
+ EXPECT_EQ("XX", conf2.getConfParam<std::string>("n", "XX"));
+ DO_EXPECT(cbs, false, false);
+
+ conf2.checkConfParam("t", "Test");
+ DO_EXPECT(cbs, false, false);
+ } // ConfigTree destroyed here
+ DO_EXPECT(cbs, false, false);
+}
+
--
GitLab