From 8c0d16a068572beb4080092a3fe3497fe4bfee15 Mon Sep 17 00:00:00 2001 From: Tristan Penman Date: Mon, 6 Jul 2020 11:47:58 +1000 Subject: [PATCH] Cosmetic improvements for std_string_adapter.hpp, test_json_pointer.cpp and test_poly_constraint.cpp --- .../valijson/adapters/std_string_adapter.hpp | 158 +++++++++--------- tests/test_json_pointer.cpp | 125 +++++++------- tests/test_poly_constraint.cpp | 22 +-- 3 files changed, 143 insertions(+), 162 deletions(-) diff --git a/include/valijson/adapters/std_string_adapter.hpp b/include/valijson/adapters/std_string_adapter.hpp index 99d09ab..fca2b93 100644 --- a/include/valijson/adapters/std_string_adapter.hpp +++ b/include/valijson/adapters/std_string_adapter.hpp @@ -35,13 +35,13 @@ public: typedef StdStringArrayValueIterator const_iterator; typedef StdStringArrayValueIterator iterator; - StdStringArray() { } + StdStringArray() = default; StdStringArrayValueIterator begin() const; StdStringArrayValueIterator end() const; - size_t size() const + static size_t size() { return 0; } @@ -53,7 +53,7 @@ public: typedef StdStringObjectMemberIterator const_iterator; typedef StdStringObjectMemberIterator iterator; - StdStringObject() { } + StdStringObject() = default; StdStringObjectMemberIterator begin() const; @@ -61,7 +61,7 @@ public: StdStringObjectMemberIterator find(const std::string &propertyName) const; - size_t size() const + static size_t size() { return 0; } @@ -70,15 +70,15 @@ public: class StdStringFrozenValue: public FrozenValue { public: - explicit StdStringFrozenValue(const std::string &source) - : value(source) { } + explicit StdStringFrozenValue(std::string source) + : value(std::move(source)) { } - virtual FrozenValue * clone() const + FrozenValue * clone() const override { return new StdStringFrozenValue(value); } - virtual bool equalTo(const Adapter &other, bool strict) const; + bool equalTo(const Adapter &other, bool strict) const override; private: std::string value; @@ -91,15 +91,15 @@ public: typedef StdStringObject Object; typedef StdStringObjectMember ObjectMember; - StdStringAdapter(const std::string &value) - : value(value) { } + explicit StdStringAdapter(const std::string &value) + : m_value(value) { } - virtual bool applyToArray(ArrayValueCallback fn) const + bool applyToArray(ArrayValueCallback fn) const override { return maybeArray(); } - virtual bool applyToObject(ObjectMemberCallback fn) const + bool applyToObject(ObjectMemberCallback fn) const override { return maybeObject(); } @@ -107,40 +107,40 @@ public: StdStringArray asArray() const { if (maybeArray()) { - return StdStringArray(); + return {}; } throw std::runtime_error("String value cannot be cast to array"); } - virtual bool asBool() const + bool asBool() const override { return true; } - virtual bool asBool(bool &result) const + bool asBool(bool &result) const override { result = true; return true; } - virtual double asDouble() const + double asDouble() const override { return 0; } - virtual bool asDouble(double &result) const + bool asDouble(double &result) const override { result = 0; return true; } - virtual int64_t asInteger() const + int64_t asInteger() const override { return 0; } - virtual bool asInteger(int64_t &result) const + bool asInteger(int64_t &result) const override { result = 0; return true; @@ -149,179 +149,179 @@ public: StdStringObject asObject() const { if (maybeObject()) { - return StdStringObject(); + return {}; } throw std::runtime_error("String value cannot be cast to object"); } - virtual std::string asString() const + std::string asString() const override { - return value; + return m_value; } - virtual bool asString(std::string &result) const + bool asString(std::string &result) const override { - result = value; + result = m_value; return true; } - virtual bool equalTo(const Adapter &other, bool strict) const + bool equalTo(const Adapter &other, bool strict) const override { if (strict && !other.isString()) { return false; } - return value.compare(other.asString()) == 0; + return m_value == other.asString(); } - virtual FrozenValue* freeze() const + FrozenValue* freeze() const override { - return new StdStringFrozenValue(value); + return new StdStringFrozenValue(m_value); } - StdStringArray getArray() const + static StdStringArray getArray() { throw std::runtime_error("Not supported"); } - virtual size_t getArraySize() const + size_t getArraySize() const override { throw std::runtime_error("Not supported"); } - virtual bool getArraySize(size_t &result) const + bool getArraySize(size_t &result) const override { throw std::runtime_error("Not supported"); } - virtual bool getBool() const + bool getBool() const override { throw std::runtime_error("Not supported"); } - virtual bool getBool(bool &result) const + bool getBool(bool &result) const override { throw std::runtime_error("Not supported"); } - virtual double getDouble() const + double getDouble() const override { throw std::runtime_error("Not supported"); } - virtual bool getDouble(double &result) const + bool getDouble(double &result) const override { throw std::runtime_error("Not supported"); } - virtual int64_t getInteger() const + int64_t getInteger() const override { throw std::runtime_error("Not supported"); } - virtual bool getInteger(int64_t &result) const + bool getInteger(int64_t &result) const override { throw std::runtime_error("Not supported"); } - virtual double getNumber() const + double getNumber() const override { throw std::runtime_error("Not supported"); } - virtual bool getNumber(double &result) const + bool getNumber(double &result) const override { throw std::runtime_error("Not supported"); } - virtual size_t getObjectSize() const + size_t getObjectSize() const override { throw std::runtime_error("Not supported"); } - virtual bool getObjectSize(size_t &result) const + bool getObjectSize(size_t &result) const override { throw std::runtime_error("Not supported"); } - virtual std::string getString() const + std::string getString() const override { - return value; + return m_value; } - virtual bool getString(std::string &result) const + bool getString(std::string &result) const override { - result = value; + result = m_value; return true; } - virtual bool hasStrictTypes() const + bool hasStrictTypes() const override { return true; } - virtual bool isArray() const + bool isArray() const override { return false; } - virtual bool isBool() const + bool isBool() const override { return false; } - virtual bool isDouble() const + bool isDouble() const override { return false; } - virtual bool isInteger() const + bool isInteger() const override { return false; } - virtual bool isNull() const + bool isNull() const override { return false; } - virtual bool isNumber() const + bool isNumber() const override { return false; } - virtual bool isObject() const + bool isObject() const override { return false; } - virtual bool isString() const + bool isString() const override { return true; } - virtual bool maybeArray() const + bool maybeArray() const override { return false; } - virtual bool maybeBool() const + bool maybeBool() const override { - return value.compare("true") == 0 || value.compare("false") == 0; + return m_value == "true" || m_value == "false"; } - virtual bool maybeDouble() const + bool maybeDouble() const override { - const char *b = value.c_str(); - char *e = NULL; + const char *b = m_value.c_str(); + char *e = nullptr; strtod(b, &e); - return e != b && e == b + value.length(); + return e != b && e == b + m_value.length(); } - virtual bool maybeInteger() const + bool maybeInteger() const override { - std::istringstream i(value); + std::istringstream i(m_value); int64_t x; char c; if (!(i >> x) || i.get(c)) { @@ -331,29 +331,26 @@ public: return true; } - virtual bool maybeNull() const + bool maybeNull() const override { - return value.size() == 0; + return m_value.empty(); } - virtual bool maybeObject() const + bool maybeObject() const override { - return value.size() == 0; + return m_value.empty(); } - virtual bool maybeString() const + bool maybeString() const override { return true; } private: - const std::string &value; + const std::string &m_value; }; -class StdStringArrayValueIterator: - public std::iterator< - std::bidirectional_iterator_tag, - StdStringAdapter> +class StdStringArrayValueIterator: public std::iterator { public: StdStringAdapter operator*() const @@ -399,18 +396,15 @@ public: inline StdStringArrayValueIterator StdStringArray::begin() const { - return StdStringArrayValueIterator(); + return {}; } inline StdStringArrayValueIterator StdStringArray::end() const { - return StdStringArrayValueIterator(); + return {}; } -class StdStringObjectMemberIterator: - public std::iterator< - std::bidirectional_iterator_tag, - StdStringObjectMember> +class StdStringObjectMemberIterator: public std::iterator { public: StdStringObjectMember operator*() const @@ -451,17 +445,17 @@ public: inline StdStringObjectMemberIterator StdStringObject::begin() const { - return StdStringObjectMemberIterator(); + return {}; } inline StdStringObjectMemberIterator StdStringObject::end() const { - return StdStringObjectMemberIterator(); + return {}; } inline StdStringObjectMemberIterator StdStringObject::find(const std::string &propertyName) const { - return StdStringObjectMemberIterator(); + return {}; } template<> diff --git a/tests/test_json_pointer.cpp b/tests/test_json_pointer.cpp index 9b2197f..641ae11 100644 --- a/tests/test_json_pointer.cpp +++ b/tests/test_json_pointer.cpp @@ -9,8 +9,7 @@ using valijson::adapters::RapidJsonAdapter; using valijson::internal::json_pointer::resolveJsonPointer; -typedef rapidjson::MemoryPoolAllocator - RapidJsonCrtAllocator; +typedef rapidjson::MemoryPoolAllocator RapidJsonCrtAllocator; class TestJsonPointer : public testing::Test { @@ -19,9 +18,6 @@ class TestJsonPointer : public testing::Test struct JsonPointerTestCase { - JsonPointerTestCase(const std::string &description) - : description(description) { } - /// Description of test case std::string description; @@ -43,77 +39,77 @@ std::vector > std::vector testCases; - TestCase testCase = std::make_shared( - "Resolving '#' should cause an exception to be thrown"); + TestCase testCase = std::make_shared(); + testCase->description = "Resolving '#' should cause an exception to be thrown"; testCase->value.SetNull(); testCase->jsonPointer = "#"; - testCase->expectedValue = NULL; + testCase->expectedValue = nullptr; testCases.push_back(testCase); - testCase = std::make_shared( - "Resolving an empty string should return the root node"); + testCase = std::make_shared(); + testCase->description = "Resolving an empty string should return the root node"; testCase->value.SetNull(); testCase->jsonPointer = ""; testCase->expectedValue = &testCase->value; testCases.push_back(testCase); - testCase = std::make_shared( - "Resolving '/' should return the root node"); + testCase = std::make_shared(); + testCase->description = "Resolving '/' should return the root node"; testCase->value.SetNull(); testCase->jsonPointer = "/"; testCase->expectedValue = &testCase->value; testCases.push_back(testCase); - testCase = std::make_shared( - "Resolving '//' should return the root node"); + testCase = std::make_shared(); + testCase->description = "Resolving '//' should return the root node"; testCase->value.SetNull(); testCase->jsonPointer = "//"; testCase->expectedValue = &testCase->value; testCases.push_back(testCase); - testCase = std::make_shared( - "Resolve '/test' in object containing one member named 'test'"); + testCase = std::make_shared(); + testCase->description = "Resolve '/test' in object containing one member named 'test'"; testCase->value.SetObject(); testCase->value.AddMember("test", "test", allocator); testCase->jsonPointer = "/test"; testCase->expectedValue = &testCase->value.FindMember("test")->value; testCases.push_back(testCase); - testCase = std::make_shared( - "Resolve '/test/' in object containing one member named 'test'"); + testCase = std::make_shared(); + testCase->description = "Resolve '/test/' in object containing one member named 'test'"; testCase->value.SetObject(); testCase->value.AddMember("test", "test", allocator); testCase->jsonPointer = "/test/"; testCase->expectedValue = &testCase->value.FindMember("test")->value; testCases.push_back(testCase); - testCase = std::make_shared( - "Resolve '//test//' in object containing one member named 'test'"); + testCase = std::make_shared(); + testCase->description = "Resolve '//test//' in object containing one member named 'test'"; testCase->value.SetObject(); testCase->value.AddMember("test", "test", allocator); testCase->jsonPointer = "//test//"; testCase->expectedValue = &testCase->value.FindMember("test")->value; testCases.push_back(testCase); - testCase = std::make_shared( - "Resolve '/missing' in object containing one member name 'test'"); + testCase = std::make_shared(); + testCase->description = "Resolve '/missing' in object containing one member name 'test'"; testCase->value.SetObject(); testCase->value.AddMember("test", "test", allocator); testCase->jsonPointer = "/missing"; - testCase->expectedValue = NULL; + testCase->expectedValue = nullptr; testCases.push_back(testCase); { rapidjson::Value nonemptyString; nonemptyString.SetString("hello, world"); - testCase = std::make_shared( - "Resolve '/value/foo' fails because 'value' is not an object (but a non empty string)"); + testCase = std::make_shared(); + testCase->description = "Resolve '/value/foo' fails because 'value' is not an object (but a non empty string)"; testCase->value.SetObject(); testCase->value.AddMember("value", nonemptyString, allocator); testCase->jsonPointer = "/value/bar"; testCase->expectedValue = &testCase->value; - testCase->expectedValue = NULL; + testCase->expectedValue = nullptr; testCases.push_back(testCase); } @@ -121,12 +117,12 @@ std::vector > rapidjson::Value emptyString; emptyString.SetString(""); - testCase = std::make_shared( - "Resolve '/empty/after_empty' fails because 'empty' is an empty string"); + testCase = std::make_shared(); + testCase->description = "Resolve '/empty/after_empty' fails because 'empty' is an empty string"; testCase->value.SetObject(); testCase->value.AddMember("empty", emptyString, allocator); testCase->jsonPointer = "/empty/after_empty"; - testCase->expectedValue = NULL; + testCase->expectedValue = nullptr; testCases.push_back(testCase); } @@ -137,9 +133,8 @@ std::vector > testArray.PushBack("test1", allocator); testArray.PushBack("test2", allocator); - testCase = std::make_shared( - "Resolve '/test/0' in object containing one member containing " - "an array with 3 elements"); + testCase = std::make_shared(); + testCase->description = "Resolve '/test/0' in object containing one member containing an array with 3 elements"; testCase->value.SetObject(); testCase->value.AddMember("test", testArray, allocator); testCase->jsonPointer = "/test/0"; @@ -154,9 +149,8 @@ std::vector > testArray.PushBack("test1", allocator); testArray.PushBack("test2", allocator); - testCase = std::make_shared( - "Resolve '/test/1' in object containing one member containing " - "an array with 3 elements"); + testCase = std::make_shared(); + testCase->description = "Resolve '/test/1' in object containing one member containing an array with 3 elements"; testCase->value.SetObject(); testCase->value.AddMember("test", testArray, allocator); testCase->jsonPointer = "/test/1"; @@ -171,9 +165,8 @@ std::vector > testArray.PushBack("test1", allocator); testArray.PushBack("test2", allocator); - testCase = std::make_shared( - "Resolve '/test/2' in object containing one member containing " - "an array with 3 elements"); + testCase = std::make_shared(); + testCase->description = "Resolve '/test/2' in object containing one member containing an array with 3 elements"; testCase->value.SetObject(); testCase->value.AddMember("test", testArray, allocator); testCase->jsonPointer = "/test/2"; @@ -188,13 +181,13 @@ std::vector > testArray.PushBack("test1", allocator); testArray.PushBack("test2", allocator); - testCase = std::make_shared( - "Resolving '/test/3' in object containing one member containing " - "an array with 3 elements should throw an exception"); + testCase = std::make_shared(); + testCase->description = "Resolving '/test/3' in object containing one member containing " + "an array with 3 elements should throw an exception"; testCase->value.SetObject(); testCase->value.AddMember("test", testArray, allocator); testCase->jsonPointer = "/test/3"; - testCase->expectedValue = NULL; + testCase->expectedValue = nullptr; testCases.push_back(testCase); } @@ -219,12 +212,12 @@ std::vector > testArray.PushBack("test1", allocator); testArray.PushBack("test2", allocator); - testCase = std::make_shared( - "Resolving '/test/-' in object containing one member containing " - "an array with 3 elements should throw an exception"); + testCase = std::make_shared(); + testCase->description = "Resolving '/test/-' in object containing one member containing " + "an array with 3 elements should throw an exception"; testCase->value.SetNull(); testCase->jsonPointer = "/test/-"; - testCase->expectedValue = NULL; + testCase->expectedValue = nullptr; testCases.push_back(testCase); } @@ -247,9 +240,9 @@ std::vector > rapidjson::Value value; value.SetDouble(10.); - testCase = std::make_shared( - "Resolving '/hello~1world' in object containing one member named " - "'hello/world' should return the associated value"); + testCase = std::make_shared(); + testCase->description = "Resolving '/hello~1world' in object containing one member named " + "'hello/world' should return the associated value"; testCase->value.SetObject(); testCase->value.AddMember("hello/world", value, allocator); testCase->jsonPointer = "/hello~1world"; @@ -261,9 +254,9 @@ std::vector > rapidjson::Value value; value.SetDouble(10.); - testCase = std::make_shared( - "Resolving '/hello~0world' in object containing one member named " - "'hello~world' should return the associated value"); + testCase = std::make_shared(); + testCase->description = "Resolving '/hello~0world' in object containing one member named " + "'hello~world' should return the associated value"; testCase->value.SetObject(); testCase->value.AddMember("hello~world", value, allocator); testCase->jsonPointer = "/hello~0world"; @@ -275,9 +268,9 @@ std::vector > rapidjson::Value value; value.SetDouble(10.); - testCase = std::make_shared( - "Resolving '/hello~01world' in object containing one member named " - "'hello~1world' should return the associated value"); + testCase = std::make_shared(); + testCase->description = "Resolving '/hello~01world' in object containing one member named " + "'hello~1world' should return the associated value"; testCase->value.SetObject(); testCase->value.AddMember("hello~1world", value, allocator); testCase->jsonPointer = "/hello~01world"; @@ -297,21 +290,15 @@ TEST_F(TestJsonPointer, JsonPointerTestCases) TestCases testCases = testCasesForSingleLevelObjectPointers(allocator); - for (TestCases::const_iterator itr = testCases.begin(); - itr != testCases.end(); ++itr) { - const std::string &jsonPointer = (*itr)->jsonPointer; - const RapidJsonAdapter valueAdapter((*itr)->value); - if ((*itr)->expectedValue) { - const RapidJsonAdapter expectedAdapter(*((*itr)->expectedValue)); - const RapidJsonAdapter actualAdapter = - resolveJsonPointer(valueAdapter, jsonPointer); - EXPECT_TRUE(actualAdapter.equalTo(expectedAdapter, true)) << - (*itr)->description; + for (const auto & testCase : testCases) { + const std::string &jsonPointer = testCase->jsonPointer; + const RapidJsonAdapter valueAdapter(testCase->value); + if (testCase->expectedValue) { + const RapidJsonAdapter expectedAdapter(*(testCase->expectedValue)); + const RapidJsonAdapter actualAdapter = resolveJsonPointer(valueAdapter, jsonPointer); + EXPECT_TRUE(actualAdapter.equalTo(expectedAdapter, true)) << testCase->description; } else { - EXPECT_THROW( - resolveJsonPointer(valueAdapter, jsonPointer), - std::runtime_error) << - (*itr)->description; + EXPECT_THROW(resolveJsonPointer(valueAdapter, jsonPointer), std::runtime_error) << testCase->description; } } } diff --git a/tests/test_poly_constraint.cpp b/tests/test_poly_constraint.cpp index 50c409a..fa69aa7 100644 --- a/tests/test_poly_constraint.cpp +++ b/tests/test_poly_constraint.cpp @@ -17,27 +17,28 @@ using valijson::adapters::RapidJsonAdapter; class StubPolyConstraint : public valijson::constraints::PolyConstraint { - bool shouldValidate; + bool m_shouldValidate; public: - StubPolyConstraint(bool shouldValidate) - : shouldValidate(shouldValidate) { } + explicit StubPolyConstraint(bool shouldValidate) + : m_shouldValidate(shouldValidate) { } - virtual Constraint * cloneInto(void *ptr) const + Constraint * cloneInto(void *ptr) const override { - return new (ptr) StubPolyConstraint(shouldValidate); + return new (ptr) StubPolyConstraint(m_shouldValidate); } - virtual size_t sizeOf() const + size_t sizeOf() const override { return sizeof(StubPolyConstraint); } - virtual bool validate(const Adapter &target, + bool validate( + const Adapter &target, const std::vector &context, - ValidationResults *results) const + ValidationResults *results) const override { - if (shouldValidate) { + if (m_shouldValidate) { return true; } @@ -92,6 +93,5 @@ TEST_F(TestPolyConstraint, ValidationCanFail) ValidationResults::Error error; EXPECT_TRUE(results.popError(error)); - EXPECT_STREQ("StubPolyConstraint intentionally failed validation", - error.description.c_str()); + EXPECT_STREQ("StubPolyConstraint intentionally failed validation", error.description.c_str()); }