From 3e69abdfa06bd417eb7bccf026d223ebe4be6c67 Mon Sep 17 00:00:00 2001 From: Riyyi Date: Sat, 16 Jul 2022 00:51:32 +0200 Subject: [PATCH] Util: Implement copy-and-swap idiom and the Rule of Five in Value --- src/util/json/array.h | 1 + src/util/json/object.h | 1 + src/util/json/tojson.h | 38 +++++----- src/util/json/value.cpp | 161 ++++++++++++++++++++++++---------------- src/util/json/value.h | 22 ++++-- 5 files changed, 131 insertions(+), 92 deletions(-) diff --git a/src/util/json/array.h b/src/util/json/array.h index df5c6d8..57a768c 100644 --- a/src/util/json/array.h +++ b/src/util/json/array.h @@ -30,6 +30,7 @@ public: { } + void clear() { m_elements.clear(); } void emplace_back(Value element); void reserve(size_t size) { m_elements.reserve(size); } diff --git a/src/util/json/object.h b/src/util/json/object.h index cf3a929..2dfb0d0 100644 --- a/src/util/json/object.h +++ b/src/util/json/object.h @@ -27,6 +27,7 @@ public: { } + void clear() { m_members.clear(); } void emplace(const std::string& name, Value value); Value& operator[](const std::string& name); diff --git a/src/util/json/tojson.h b/src/util/json/tojson.h index 40b3523..d2045bb 100644 --- a/src/util/json/tojson.h +++ b/src/util/json/tojson.h @@ -26,7 +26,7 @@ struct jsonConstructor { template static void construct(Json& json, bool boolean) { - json.clear(); + json.destroy(); json.m_type = Json::Type::Bool; json.m_value.boolean = boolean; } @@ -34,7 +34,7 @@ struct jsonConstructor { template static void construct(Json& json, int number) { - json.clear(); + json.destroy(); json.m_type = Json::Type::Number; json.m_value.number = (double)number; } @@ -42,7 +42,7 @@ struct jsonConstructor { template static void construct(Json& json, double number) { - json.clear(); + json.destroy(); json.m_type = Json::Type::Number; json.m_value.number = number; } @@ -50,7 +50,7 @@ struct jsonConstructor { template static void construct(Json& json, const char* string) { - json.clear(); + json.destroy(); json.m_type = Json::Type::String; json.m_value.string = new std::string(string); } @@ -58,7 +58,7 @@ struct jsonConstructor { template static void construct(Json& json, const std::string& string) { - json.clear(); + json.destroy(); json.m_type = Json::Type::String; json.m_value.string = new std::string(string); } @@ -66,7 +66,7 @@ struct jsonConstructor { template static void construct(Json& json, const Array& array) { - json.clear(); + json.destroy(); json.m_type = Json::Type::Array; json.m_value.array = new Array(array); } @@ -74,7 +74,7 @@ struct jsonConstructor { template static void construct(Json& json, const std::vector& array) { - json.clear(); + json.destroy(); json.m_type = Json::Type::Array; json.m_value.array = new Array; json.m_value.array->reserve(array.size()); @@ -86,7 +86,7 @@ struct jsonConstructor { template static void construct(Json& json, const Object& object) { - json.clear(); + json.destroy(); json.m_type = Json::Type::Object; json.m_value.object = new Object(object); } @@ -94,7 +94,7 @@ struct jsonConstructor { template static void construct(Json& json, const std::map& object) { - json.clear(); + json.destroy(); json.m_type = Json::Type::Object; json.m_value.object = new Object; for (const auto& [name, value] : object) { @@ -105,7 +105,7 @@ struct jsonConstructor { template static void construct(Json& json, const std::unordered_map& object) { - json.clear(); + json.destroy(); json.m_type = Json::Type::Object; json.m_value.object = new Object; for (const auto& [name, value] : object) { @@ -143,14 +143,14 @@ constexpr const auto& toJson = Detail::staticConst; // N // Customization Points // https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4381.html -// Json::fromJson is a function object, the type of which is -// Json::Detail::fromJsonFunction. In the Json::Detail namespace are the -// fromJson free functions. The function call operator of fromJsonFunction makes -// an unqualified call to fromJson which, since it shares the Detail namespace -// with the fromJson free functions, will consider those in addition to any -// overloads that are found by argument-dependent lookup. +// Json::toJson is a function object, the type of which is +// Json::Detail::toJsonFunction. In the Json::Detail namespace are the toJson +// free functions. The function call operator of toJsonFunction makes an +// unqualified call to toJson which, since it shares the Detail namespace with +// the toJson free functions, will consider those in addition to any overloads +// that are found by argument-dependent lookup. // Variable templates are linked externally, therefor every translation unit -// will see the same address for Detail::staticConst. -// Since Json::fromJson is a reference to the variable template, it too will -// have the same address in all translation units. +// will see the same address for Detail::staticConst. +// Since Json::toJson is a reference to the variable template, it too will have +// the same address in all translation units. diff --git a/src/util/json/value.cpp b/src/util/json/value.cpp index 7ec2980..438f3ed 100644 --- a/src/util/json/value.cpp +++ b/src/util/json/value.cpp @@ -9,7 +9,7 @@ #include // uint32_t #include // istream, ostream #include -#include // move +#include // move, swap #include "util/json/array.h" #include "util/json/job.h" @@ -27,7 +27,26 @@ Value::Value(std::nullptr_t) Value::Value(Type type) : m_type(type) { - create(); + switch (m_type) { + case Type::Bool: + m_value.boolean = false; + break; + case Type::Number: + m_value.number = 0.0; + break; + case Type::String: + m_value.string = new std::string; + break; + case Type::Array: + m_value.array = new Array; + break; + case Type::Object: + m_value.object = new Object; + break; + case Type::Null: + default: + break; + } } Value::Value(const std::initializer_list& values) @@ -48,36 +67,94 @@ Value::Value(const std::initializer_list& values) for (auto& value : values) { m_value.object->emplace(std::move(*value[0].m_value.string), - std::move(value[1])); + std::move(value[1])); } } } // Copy constructor Value::Value(const Value& other) + : m_type(other.m_type) { - copyFrom(other); + switch (m_type) { + case Type::Bool: + m_value.boolean = other.m_value.boolean; + break; + case Type::Number: + m_value.number = other.m_value.number; + break; + case Type::String: + m_value.string = new std::string(*other.m_value.string); + break; + case Type::Array: + m_value.array = new Array(*other.m_value.array); + break; + case Type::Object: + m_value.object = new Object(*other.m_value.object); + break; + case Type::Null: + default: + break; + } } -// Assignment operator -Value& Value::operator=(const Value& other) +// Move constructor +Value::Value(Value&& other) noexcept + : Value(Type::Null) // Initialize via default construction { - if (this != &other) { - clear(); - copyFrom(other); - } + // Allow std::swap as a fallback on ADL failure + using std::swap; + // Unqualified call to swap, allow ADL to operate and find best match + swap(*this, other); +} + +// Copy assignment +// Move assignment +Value& Value::operator=(Value other) +{ + // Allow std::swap as a fallback on ADL failure + using std::swap; + // Unqualified call to swap, allow ADL to operate and find best match + swap(*this, other); return *this; } +void swap(Value& left, Value& right) noexcept +{ + std::swap(left.m_type, right.m_type); + std::swap(left.m_value, right.m_value); +} + // ------------------------------------------ -Value Value::parse(const std::string& input) +void Value::clear() { - Job job(input); - Value value = job.fire(); + switch (m_type) { + case Type::Bool: + m_value.boolean = false; + break; + case Type::Number: + m_value.number = 0.0; + break; + case Type::String: + m_value.string->clear(); + break; + case Type::Array: + m_value.array->clear(); + break; + case Type::Object: + m_value.object->clear(); + break; + case Type::Null: + default: + break; + } +} - return value; +Value Value::parse(const std::string& input) +{ + return Job(input).fire(); } std::string Value::dump(const uint32_t indent, const char indentCharacter) const @@ -190,45 +267,21 @@ size_t Value::size() const switch (m_type) { case Type::Null: return 0; - case Type::Bool: - case Type::Number: - case Type::String: - return 1; case Type::Array: return m_value.array->size(); case Type::Object: return m_value.object->size(); - default: - return 1; - } -} - -// ------------------------------------------ - -void Value::create() -{ - switch (m_type) { case Type::Bool: - m_value.boolean = false; - break; case Type::Number: - m_value.number = 0.0; - break; case Type::String: - m_value.string = new std::string; - break; - case Type::Array: - m_value.array = new Array; - break; - case Type::Object: - m_value.object = new Object; - break; default: - break; + return 1; } } -void Value::clear() +// ------------------------------------------ + +void Value::destroy() { switch (m_type) { case Type::String: @@ -240,31 +293,9 @@ void Value::clear() case Type::Object: delete m_value.object; break; - default: - break; - } -} - -void Value::copyFrom(const Value& other) -{ - m_type = other.m_type; - - switch (m_type) { + case Type::Null: case Type::Bool: - m_value.boolean = other.m_value.boolean; - break; case Type::Number: - m_value.number = other.m_value.number; - break; - case Type::String: - m_value.string = new std::string(*other.m_value.string); - break; - case Type::Array: - m_value.array = new Array(*other.m_value.array); - break; - case Type::Object: - m_value.object = new Object(*other.m_value.object); - break; default: break; } diff --git a/src/util/json/value.h b/src/util/json/value.h index 602f5c7..8576d15 100644 --- a/src/util/json/value.h +++ b/src/util/json/value.h @@ -38,6 +38,8 @@ public: Object, // {} }; + // -------------------------------------- + // Constructors Value(std::nullptr_t = nullptr); Value(Type type); @@ -48,20 +50,26 @@ public: toJson(*this, std::forward(value)); } - // Destructor - virtual ~Value() { clear(); } - + // Rule of Five: // Copy constructor Value(const Value& other); + // Move constructor + Value(Value&& other) noexcept; + // Copy assignment + // Move assignment + Value& operator=(Value other); + // Destructor + virtual ~Value() { destroy(); } - // Assignment operator - Value& operator=(const Value& other); + friend void swap(Value& left, Value& right) noexcept; // -------------------------------------- static Value parse(const std::string& input); std::string dump(const uint32_t indent = 0, const char indentCharacter = ' ') const; + void clear(); + void emplace_back(Value value); void emplace(const std::string& key, Value value); @@ -109,9 +117,7 @@ public: const Object& asObject() const { return *m_value.object; } private: - void create(); - void clear(); - void copyFrom(const Value& other); + void destroy(); Type m_type { Type::Null };