cleaning up and adding move constructors

include/hepnos/DataSet.hpp

@@ -31,7 +31,7 @@ class DataSet {
      * @param level Level of nesting.
      * @param fullname Full name of the DataSet.
      */
-    DataSet(DataStore& ds, uint8_t level, const std::string& fullname);
+    DataSet(DataStore* ds, uint8_t level, const std::string& fullname);
 
     /**
      * @brief Constructor.
@@ -41,7 +41,7 @@ class DataSet {
      * @param container Full name of the parent DataSet ("" if no parent).
      * @param name Name of the DataSet.
      */
-    DataSet(DataStore& ds, uint8_t level, const std::string& container, const std::string& name);
+    DataSet(DataStore* ds, uint8_t level, const std::string& container, const std::string& name);
 
     /**
      * @brief Stores binary data associated with a particular key into this DataSet.

include/hepnos/DataStore.hpp

@@ -281,19 +281,24 @@ class DataStore::const_iterator {
     /**
      * @brief Constructor. Creates a const_iterator pointing
      * to an invalid DataSet.
+     */
+    const_iterator();
+
+    /**
+     * @brief Constructor. Creates a const_iterator pointing
+     * to a given DataSet. The DataSet may or may not be valid. 
      *
-     * @param ds DataStore creating this const_iterator.
+     * @param current DataSet to make the const_iterator point to.
      */
-    const_iterator(DataStore& ds);
+    const_iterator(const DataSet& current);
 
     /**
      * @brief Constructor. Creates a const_iterator pointing
      * to a given DataSet. The DataSet may or may not be valid. 
      *
-     * @param ds DataStore creating this const_iterator.
      * @param current DataSet to make the const_iterator point to.
      */
-    const_iterator(DataStore& ds, const DataSet& current);
+    const_iterator(DataSet&& current);
 
     public:
 
@@ -408,20 +413,26 @@ class DataStore::iterator : public DataStore::const_iterator {
     /**
      * @brief Constructor. Builds an iterator pointing to an
      * invalid DataSet.
+     */
+    iterator();
+
+    /**
+     * @brief Constructor. Builds an iterator pointing to
+     * an existing DataSet. The DataSet may or may not be
+     * valid.
      *
-     * @param ds DataStore creating the iterator.
+     * @param current DataSet to point to.
      */
-    iterator(DataStore& ds);
+    iterator(const DataSet& current);
 
     /**
      * @brief Constructor. Builds an iterator pointing to
      * an existing DataSet. The DataSet may or may not be
      * valid.
      *
-     * @param ds DataStore creating the iterator.
      * @param current DataSet to point to.
      */
-    iterator(DataStore& ds, const DataSet& current);
+    iterator(DataSet&& current);
 
     public:
 

src/DataSet.cpp

@@ -21,8 +21,8 @@ class DataSet::Impl {
 DataSet::DataSet()
 : m_impl(std::make_unique<DataSet::Impl>(nullptr, 0, "", "")) {}
 
-DataSet::DataSet(DataStore& ds, uint8_t level, const std::string& fullname)
-: m_impl(std::make_unique<DataSet::Impl>(&ds, level, "", "")) {
+DataSet::DataSet(DataStore* ds, uint8_t level, const std::string& fullname)
+: m_impl(std::make_unique<DataSet::Impl>(ds, level, "", "")) {
     size_t p = fullname.find_last_of('/');
     if(p == std::string::npos) {
         m_impl->m_name = fullname;
@@ -32,8 +32,8 @@ DataSet::DataSet(DataStore& ds, uint8_t level, const std::string& fullname)
     }
 }
 
-DataSet::DataSet(DataStore& ds, uint8_t level, const std::string& container, const std::string& name) 
-: m_impl(std::make_unique<DataSet::Impl>(&ds, level, container, name)) {}
+DataSet::DataSet(DataStore* ds, uint8_t level, const std::string& container, const std::string& name) 
+: m_impl(std::make_unique<DataSet::Impl>(ds, level, container, name)) {}
 
 DataSet::DataSet(const DataSet& other)
 : m_impl(std::make_unique<DataSet::Impl>(*other.m_impl)) {}
@@ -57,7 +57,7 @@ DataSet DataSet::next() const {
     size_t s = m_impl->m_datastore->nextKeys(
             m_impl->m_level, m_impl->m_container, m_impl->m_name, keys, 1);
     if(s == 0) return DataSet();
-    return DataSet(*(m_impl->m_datastore), m_impl->m_level, m_impl->m_container, keys[0]);
+    return DataSet(m_impl->m_datastore, m_impl->m_level, m_impl->m_container, keys[0]);
 }
 
 bool DataSet::valid() const {
@@ -112,8 +112,9 @@ DataSet DataSet::createDataSet(const std::string& name) {
     if(name.find('/') != std::string::npos) {
         throw Exception("Invalid character '/' in dataset name");
     }
-    m_impl->m_datastore->store(m_impl->m_level+1, fullname(), name, std::vector<char>());
-    return DataSet(*(m_impl->m_datastore), 1, fullname(), name);
+    std::string parent = fullname();
+    m_impl->m_datastore->store(m_impl->m_level+1, parent, name, std::vector<char>());
+    return DataSet(m_impl->m_datastore, m_impl->m_level+1, parent, name);
 }
 
 DataSet DataSet::operator[](const std::string& datasetName) const {
@@ -127,11 +128,12 @@ DataSet::iterator DataSet::find(const std::string& datasetName) {
         throw Exception("Invalid character '/' in dataset name");
     }
     std::vector<char> data;
-    bool b = m_impl->m_datastore->load(m_impl->m_level+1, fullname(), datasetName, data);
+    std::string parent = fullname();
+    bool b = m_impl->m_datastore->load(m_impl->m_level+1, parent, datasetName, data);
     if(!b) {
         return m_impl->m_datastore->end();
     }
-    return iterator(*(m_impl->m_datastore), DataSet(*(m_impl->m_datastore), 1, fullname(), datasetName));
+    return iterator(DataSet(m_impl->m_datastore, m_impl->m_level+1, parent, datasetName));
 }
 
 DataSet::const_iterator DataSet::find(const std::string& datasetName) const {
@@ -140,9 +142,9 @@ DataSet::const_iterator DataSet::find(const std::string& datasetName) const {
 }
 
 DataSet::iterator DataSet::begin() {
-    DataSet ds(*(m_impl->m_datastore), m_impl->m_level+1, fullname(),"");
+    DataSet ds(m_impl->m_datastore, m_impl->m_level+1, fullname(),"");
     ds = ds.next();
-    if(ds.valid()) return iterator(*(m_impl->m_datastore), ds);
+    if(ds.valid()) return iterator(ds);
     else return end();
 }
 
@@ -168,10 +170,10 @@ DataSet::iterator DataSet::lower_bound(const std::string& lb) {
         ++it;
         return it;
     }
-    DataSet ds(*(m_impl->m_datastore), m_impl->m_level+1, fullname(), lb2);
+    DataSet ds(m_impl->m_datastore, m_impl->m_level+1, fullname(), lb2);
     ds = ds.next();
     if(!ds.valid()) return end();
-    else return iterator(*(m_impl->m_datastore), ds);
+    else return iterator(ds);
 }
 
 DataSet::const_iterator DataSet::lower_bound(const std::string& lb) const {
@@ -180,10 +182,10 @@ DataSet::const_iterator DataSet::lower_bound(const std::string& lb) const {
 }
 
 DataSet::iterator DataSet::upper_bound(const std::string& ub) {
-    DataSet ds(*(m_impl->m_datastore), m_impl->m_level+1, fullname(), ub);
+    DataSet ds(m_impl->m_datastore, m_impl->m_level+1, fullname(), ub);
     ds = ds.next();
     if(!ds.valid()) return end();
-    else return iterator(*(m_impl->m_datastore), ds);
+    else return iterator(ds);
 }
 
 DataSet::const_iterator DataSet::upper_bound(const std::string& ub) const {

src/DataStore.cpp

@@ -19,7 +19,6 @@ namespace hepnos {
 class DataStore::Impl {
     public:
 
-        DataStore&                           m_parent;       // parent DataStore
         margo_instance_id                    m_mid;          // Margo instance
         sdskv_client_t                       m_sdskv_client; // SDSKV client
         bake_client_t                        m_bake_client;  // BAKE client
@@ -30,14 +29,13 @@ class DataStore::Impl {
         struct ch_placement_instance*        m_chi_bake;     // ch-placement instance for BAKE
         const DataStore::iterator            m_end;          // iterator for the end() of the DataStore
 
-        Impl(DataStore& parent)
-        : m_parent(parent)
-        , m_mid(MARGO_INSTANCE_NULL)
+        Impl(DataStore* parent)
+        : m_mid(MARGO_INSTANCE_NULL)
         , m_sdskv_client(SDSKV_CLIENT_NULL)
         , m_chi_sdskv(nullptr)
         , m_bake_client(BAKE_CLIENT_NULL)
         , m_chi_bake(nullptr)
-        , m_end(parent) {}
+        , m_end() {}
 
         void init(const std::string& configFile) {
             int ret;
@@ -237,7 +235,7 @@ class DataStore::Impl {
 ////////////////////////////////////////////////////////////////////////////////////////////
 
 DataStore::DataStore(const std::string& configFile) 
-: m_impl(std::make_unique<DataStore::Impl>(*this)) {
+: m_impl(std::make_unique<DataStore::Impl>(this)) {
     m_impl->init(configFile);
 }
 
@@ -260,15 +258,16 @@ DataStore::~DataStore() {
 
 DataStore::iterator DataStore::find(const std::string& datasetName) {
     int ret;
-    if(datasetName.find('/') != std::string::npos) {
-        throw Exception("Invalid character '/' in dataset name");
+    if(datasetName.find('/') != std::string::npos
+    || datasetName.find('%') != std::string::npos) {
+        throw Exception("Invalid character ('/' or '%') in dataset name");
     }
     std::vector<char> data;
     bool b = load(1, "", datasetName, data);
     if(!b) {
         return m_impl->m_end;
     }
-    return iterator(*this, DataSet(*this, 1, datasetName));
+    return iterator(DataSet(this, 1, datasetName));
 }
 
 DataSet DataStore::operator[](const std::string& datasetName) const {
@@ -282,9 +281,9 @@ DataStore::const_iterator DataStore::find(const std::string& datasetName) const
 }
 
 DataStore::iterator DataStore::begin() {
-    DataSet ds(*this, 1, "", "");
+    DataSet ds(this, 1, "", "");
     ds = ds.next();
-    if(ds.valid()) return iterator(*this, ds);
+    if(ds.valid()) return iterator(std::move(ds));
     else return end();
 }
 
@@ -293,7 +292,7 @@ DataStore::iterator DataStore::end() {
 }
 
 DataStore::const_iterator DataStore::cbegin() const {
-    return const_iterator(const_cast<DataStore*>(this)->begin());
+    return const_cast<DataStore*>(this)->begin();
 }
 
 DataStore::const_iterator DataStore::cend() const {
@@ -310,10 +309,10 @@ DataStore::iterator DataStore::lower_bound(const std::string& lb) {
         ++it;
         return it;
     }
-    DataSet ds(*this, 1, "", lb2);
+    DataSet ds(this, 1, "", lb2);
     ds = ds.next();
     if(!ds.valid()) return end();
-    else return iterator(*this, ds);
+    else return iterator(std::move(ds));
 }
 
 DataStore::const_iterator DataStore::lower_bound(const std::string& lb) const {
@@ -322,10 +321,10 @@ DataStore::const_iterator DataStore::lower_bound(const std::string& lb) const {
 }
 
 DataStore::iterator DataStore::upper_bound(const std::string& ub) {
-    DataSet ds(*this, 1, "", ub);
+    DataSet ds(this, 1, "", ub);
     ds = ds.next();
     if(!ds.valid()) return end();
-    else return iterator(*this, ds);
+    else return iterator(std::move(ds));
 }
 
 DataStore::const_iterator DataStore::upper_bound(const std::string& ub) const {
@@ -334,11 +333,12 @@ DataStore::const_iterator DataStore::upper_bound(const std::string& ub) const {
 }
 
 DataSet DataStore::createDataSet(const std::string& name) {
-    if(name.find('/') != std::string::npos) {
-        throw Exception("Invalid character '/' in dataset name");
+    if(name.find('/') != std::string::npos
+    || name.find('%') != std::string::npos) {
+        throw Exception("Invalid character ('/' or '%') in dataset name");
     }
     store(1, "", name, std::vector<char>());
-    return DataSet(*this, 1, "", name);
+    return DataSet(this, 1, "", name);
 }
 
 bool DataStore::load(uint8_t level, const std::string& containerName,
@@ -487,27 +487,26 @@ size_t DataStore::nextKeys(uint8_t level, const std::string& containerName,
 
 class DataStore::const_iterator::Impl {
     public:
-        DataStore* m_datastore;
         DataSet    m_current_dataset;
 
-        Impl(DataStore& ds)
-        : m_datastore(&ds)
-        , m_current_dataset()
+        Impl()
+        : m_current_dataset()
         {}
 
-        Impl(DataStore& ds, const DataSet& dataset)
-        : m_datastore(&ds)
-        , m_current_dataset(dataset)
+        Impl(const DataSet& dataset)
+        : m_current_dataset(dataset)
+        {}
+
+        Impl(DataSet&& dataset)
+        : m_current_dataset(std::move(dataset))
         {}
 
         Impl(const Impl& other)
-        : m_datastore(other.m_datastore)
-        , m_current_dataset(other.m_current_dataset) 
+        : m_current_dataset(other.m_current_dataset) 
         {}
 
         bool operator==(const Impl& other) const {
-            return m_datastore == other.m_datastore
-                && m_current_dataset == other.m_current_dataset;
+            return m_current_dataset == other.m_current_dataset;
         }
 };
 
@@ -515,11 +514,14 @@ class DataStore::const_iterator::Impl {
 // DataStore::const_iterator::Impl implementation
 ////////////////////////////////////////////////////////////////////////////////////////////
 
-DataStore::const_iterator::const_iterator(DataStore& ds)
-: m_impl(std::make_unique<Impl>(ds)) {}
+DataStore::const_iterator::const_iterator()
+: m_impl(std::make_unique<Impl>()) {}
 
-DataStore::const_iterator::const_iterator(DataStore& ds, const DataSet& dataset)
-: m_impl(std::make_unique<Impl>(ds, dataset)) {}
+DataStore::const_iterator::const_iterator(const DataSet& dataset)
+: m_impl(std::make_unique<Impl>(dataset)) {}
+
+DataStore::const_iterator::const_iterator(DataSet&& dataset)
+: m_impl(std::make_unique<Impl>(std::move(dataset))) {}
 
 DataStore::const_iterator::~const_iterator() {}
 
@@ -582,11 +584,14 @@ bool DataStore::const_iterator::operator!=(const self_type& rhs) const {
 // DataStore::iterator implementation
 ////////////////////////////////////////////////////////////////////////////////////////////
 
-DataStore::iterator::iterator(DataStore& ds, const DataSet& current)
-: const_iterator(ds, current) {}
+DataStore::iterator::iterator(const DataSet& current)
+: const_iterator(current) {}
+
+DataStore::iterator::iterator(DataSet&& current)
+: const_iterator(std::move(current)) {}
 
-DataStore::iterator::iterator(DataStore& ds)
-: const_iterator(ds) {}
+DataStore::iterator::iterator()
+: const_iterator() {}
 
 DataStore::iterator::~iterator() {}