From 1acc4428b9897c2495f48d4fc372899f21291daa Mon Sep 17 00:00:00 2001
From: Timm Fitschen <t.fitschen@indiscale.com>
Date: Fri, 15 Oct 2021 10:59:13 +0200
Subject: [PATCH] MOre tests and styling

---
 include/caosdb/data_type.h |  5 +++--
 include/caosdb/entity.h    | 35 ++++++++++++++----------------
 test/test_data_type.cpp    | 44 ++++++++++++++++++++++++++++++++------
 test/test_entity.cpp       |  3 +--
 4 files changed, 58 insertions(+), 29 deletions(-)

diff --git a/include/caosdb/data_type.h b/include/caosdb/data_type.h
index ced4399..771ca8c 100644
--- a/include/caosdb/data_type.h
+++ b/include/caosdb/data_type.h
@@ -158,8 +158,7 @@ public:
   DataType(ProtoDataType *wrapped) : ScalarProtoMessageWrapper<ProtoDataType>(wrapped) {}
   DataType() : ScalarProtoMessageWrapper<ProtoDataType>(static_cast<ProtoDataType *>(nullptr)) {}
   /**
-   * Create an AtomicDataType typed DataType.  For references, use the
-   * std::string constructor.
+   * Create an AtomicDataType typed DataType.  For references, use the std::string constructor.
    */
   DataType(AtomicDataType data_type, bool list_type = false)
     : ScalarProtoMessageWrapper<ProtoDataType>() {
@@ -182,6 +181,8 @@ public:
     }
   }
 
+  ~DataType() = default;
+
   inline static auto ListOf(const AtomicDataType &atomic_data_type) -> DataType {
     return DataType(atomic_data_type, true);
   }
diff --git a/include/caosdb/entity.h b/include/caosdb/entity.h
index 0c8892d..0e34e43 100644
--- a/include/caosdb/entity.h
+++ b/include/caosdb/entity.h
@@ -388,20 +388,19 @@ public:
   /**
    * Copy constructor.
    */
-  inline Parent(const Parent &other) : Parent(ProtoMessageWrapper<ProtoParent>::CopyProtoMessage(other.wrapped)) { }
+  inline Parent(const Parent &other)
+    : Parent(ProtoMessageWrapper<ProtoParent>::CopyProtoMessage(other.wrapped)) {}
 
   /**
    * Move constructor.
    */
-  inline Parent(Parent &&other) : Parent(other.wrapped) {
-    other.wrapped = nullptr;
-  }
+  inline Parent(Parent &&other) : Parent(other.wrapped) { other.wrapped = nullptr; }
 
   /**
    * Copy assignment operator.
    */
   inline auto operator=(const Parent &other) -> Parent & {
-    if(this != &other) {
+    if (this != &other) {
       this->wrapped->CopyFrom(*other.wrapped);
     }
     return *this;
@@ -416,8 +415,6 @@ public:
     return *this;
   }
 
-
-
   /**
    * Return the id of the parent entity.
    */
@@ -684,17 +681,17 @@ private:
  * Overview of the Constructors:
  *
  * <li> Entity() - Calls Entity(ProtoEntity *) with a fresh ProtoEntity
- * <li> Entity(Entity) - Copy constructor, calls Entity(ProtoEntity *) after
- * copying wrapped ProtoEntity of the original, then also copies all Messages.
+ * <li> Entity(const Entity&) - Copy constructor, calls Entity(ProtoEntity *) after
+ *   copying wrapped ProtoEntity of the original, then also copies all Messages.
  * <li> Entity(ProtoEntity *) - The workhorse of the constructors. Initializes
- * everything and does not call other Entity constructors. <li>
- * Entity(EntityResponse *) - Constructor which is used by the Transaction class
- * to create an Entity from the server's response, calls Entity(ProtoEntity).
- * <li> Entity(IdResponse
- * *) - Constructor which is used by the Transaction class to create an Entity
- * from the servers's response. calls Entity(), then moves the data to the
- * wrapped ProtoEntity.
- *
+ *   everything and does not call other Entity constructors.
+ * <li> Entity(EntityResponse *) - Constructor which is used by the Transaction class
+ *   to create an Entity from the server's response, calls Entity(ProtoEntity).
+ * <li> Entity(IdResponse *) - Constructor which is used by the Transaction
+ *   class to create an Entity from the servers's response. calls Entity(),
+ *   then moves the data to the wrapped ProtoEntity.
+ * <li> Entity(Entity&&) - Move constructor, calls Entity(ProtoEntity *), then
+ *   moves the messages and resets the original,
  */
 class Entity : public ScalarProtoMessageWrapper<ProtoEntity> {
 public:
@@ -746,8 +743,8 @@ public:
     original.wrapped = nullptr;
     original.value.wrapped = nullptr;
     original.data_type.wrapped = nullptr;
-    this->properties = std::move(original.properties);
-    this->parents = std::move(original.parents);
+    original.properties.wrapped = nullptr;
+    original.parents.wrapped = nullptr;
     this->errors = std::move(original.errors);
     this->warnings = std::move(original.warnings);
     this->infos = std::move(original.infos);
diff --git a/test/test_data_type.cpp b/test/test_data_type.cpp
index 5ea3a29..cc59a65 100644
--- a/test/test_data_type.cpp
+++ b/test/test_data_type.cpp
@@ -136,15 +136,47 @@ TEST(test_data_type, data_type_copy_constructor) {
   // copy
   const DataType copy_data_type(data_type);
   EXPECT_EQ(copy_data_type, data_type);
+  EXPECT_EQ(dt_string, copy_data_type.ToString());
 }
 
-// TEST(test_data_type, data_type_move_constructor) {
-//}
+TEST(test_data_type, data_type_move_constructor) {
+  DataType data_type("person", true);
+  const auto dt_string = data_type.ToString();
+
+  // copy for testing
+  const DataType copy_data_type(data_type);
+  // move
+  const DataType move_data_type(std::move(data_type));
+  EXPECT_NE(data_type, copy_data_type);       // NOLINT
+  EXPECT_NE(data_type.ToString(), dt_string); // NOLINT
+
+  EXPECT_EQ(copy_data_type, move_data_type);
+  EXPECT_EQ(move_data_type.ToString(), dt_string);
+}
+
+TEST(test_data_type, data_type_copy_assignment) {
+  DataType data_type("person", true);
+  const auto dt_string = data_type.ToString();
 
-// TEST(test_data_type, data_type_copy_assignment) {
-//}
+  // copy
+  DataType copy_data_type = data_type;
+  EXPECT_EQ(copy_data_type, data_type);
+  EXPECT_EQ(dt_string, copy_data_type.ToString());
+}
 
-// TEST(test_data_type, data_type_move_assignment) {
-//}
+TEST(test_data_type, data_type_move_assignment) {
+  DataType data_type("person", true);
+  const auto dt_string = data_type.ToString();
+
+  // copy for testing
+  const DataType copy_data_type(data_type);
+  // move
+  DataType move_data_type = std::move(data_type);
+  EXPECT_NE(data_type, copy_data_type);       // NOLINT
+  EXPECT_NE(data_type.ToString(), dt_string); // NOLINT
+
+  EXPECT_EQ(copy_data_type, move_data_type);
+  EXPECT_EQ(move_data_type.ToString(), dt_string);
+}
 
 } // namespace caosdb::entity
diff --git a/test/test_entity.cpp b/test/test_entity.cpp
index eb02abb..f66ee3f 100644
--- a/test/test_entity.cpp
+++ b/test/test_entity.cpp
@@ -109,8 +109,7 @@ TEST(test_entity, test_list_property_setters) {
   EXPECT_FALSE(dtype.IsAtomic()); // Should not be true anymore.
   EXPECT_FALSE(dtype.IsReference());
   EXPECT_TRUE(dtype.IsList());
-  EXPECT_NE(dtype.GetAsAtomic(),
-            AtomicDataType::DATETIME); // Should be overwritten.
+  EXPECT_NE(dtype.GetAsAtomic(), AtomicDataType::DATETIME); // Should be overwritten.
   EXPECT_TRUE(dtype.GetAsList().IsListOfAtomic());
   EXPECT_EQ(dtype.GetAsList().GetAtomicDataType(), AtomicDataType::DOUBLE);
 }
-- 
GitLab