Util: Implement copy-and-swap idiom and the Rule of Five in Value

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); }
 

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);

src/util/json/tojson.h

@@ -26,7 +26,7 @@ struct jsonConstructor {
 	template<typename Json>
 	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<typename Json>
 	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<typename Json>
 	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<typename Json>
 	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<typename Json>
 	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<typename Json>
 	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<typename Json, typename T>
 	static void construct(Json& json, const std::vector<T>& 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<typename Json>
 	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<typename Json, typename T>
 	static void construct(Json& json, const std::map<std::string, T>& 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<typename Json, typename T>
 	static void construct(Json& json, const std::unordered_map<std::string, T>& 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<Detail::toJsonFunction>; // 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::toJson is a function object, the type of which is
-// Json::Detail::fromJsonFunction. In the Json::Detail namespace are the
+// Json::Detail::toJsonFunction. In the Json::Detail namespace are the toJson
-// fromJson free functions. The function call operator of fromJsonFunction makes
+// free functions. The function call operator of toJsonFunction makes an
-// an unqualified call to fromJson which, since it shares the Detail namespace
+// unqualified call to toJson which, since it shares the Detail namespace with
-// with the fromJson free functions, will consider those in addition to any
+// the toJson free functions, will consider those in addition to any overloads
-// overloads that are found by argument-dependent lookup.
+// that are found by argument-dependent lookup.
 
 // Variable templates are linked externally, therefor every translation unit
-// will see the same address for Detail::staticConst<Detail::fromJsonFunction>.
+// will see the same address for Detail::staticConst<Detail::toJsonFunction>.
-// Since Json::fromJson is a reference to the variable template, it too will
+// Since Json::toJson is a reference to the variable template, it too will have
-// have the same address in all translation units.
+// the same address in all translation units.

src/util/json/value.cpp

@@ -9,7 +9,7 @@
 #include <cstdint>   // uint32_t
 #include <iostream>  // istream, ostream
 #include <string>
-#include <utility> // move
+#include <utility> // 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<Value>& values)
@@ -55,29 +74,87 @@ Value::Value(const std::initializer_list<Value>& values)
 
 // 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
+// Move constructor
-Value& Value::operator=(const Value& other)
+Value::Value(Value&& other) noexcept
+	: Value(Type::Null) // Initialize via default construction
 {
-	if (this != &other) {
+	// Allow std::swap as a fallback on ADL failure
-		clear();
+	using std::swap;
-		copyFrom(other);
+	// 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);
+}
+
 // ------------------------------------------
 
+void Value::clear()
+{
+	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;
+	}
+}
+
 Value Value::parse(const std::string& input)
 {
-	Job job(input);
+	return Job(input).fire();
-	Value value = job.fire();
-
-	return value;
 }
 
 std::string Value::dump(const uint32_t indent, const char indentCharacter) const
@@ -190,14 +267,13 @@ 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();
+	case Type::Bool:
+	case Type::Number:
+	case Type::String:
 	default:
 		return 1;
 	}
@@ -205,30 +281,7 @@ size_t Value::size() const
 
 // ------------------------------------------
 
-void Value::create()
+void Value::destroy()
-{
-	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;
-	}
-}
-
-void Value::clear()
 {
 	switch (m_type) {
 	case Type::String:
@@ -240,31 +293,9 @@ void Value::clear()
 	case Type::Object:
 		delete m_value.object;
 		break;
-	default:
+	case Type::Null:
-		break;
-	}
-}
-
-void Value::copyFrom(const Value& other)
-{
-	m_type = other.m_type;
-
-	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;
 	default:
 		break;
 	}

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<T>(value));
 	}
 
-	// Destructor
+	// Rule of Five:
-	virtual ~Value() { clear(); }
-
 	// 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
+	friend void swap(Value& left, Value& right) noexcept;
-	Value& operator=(const Value& other);
 
 	// --------------------------------------
 
 	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 destroy();
-	void clear();
-	void copyFrom(const Value& other);
 
 	Type m_type { Type::Null };
 

test/unit/testutiljson.cpp

@@ -28,9 +28,9 @@
 #define EXEC(x) x
 #else
 #define EXEC(x)                                   \
-	stdout = Test::TestSuite::the().outputNull(); \
+	stderr = Test::TestSuite::the().outputNull(); \
 	x;                                            \
-	stdout = Test::TestSuite::the().outputStd();
+	stderr = Test::TestSuite::the().outputErr();
 #endif
 
 std::vector<Json::Token> lex(const std::string& input)