From b0e12e8852f0458f9e37d75b541f6aaf302bbea5 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Thu, 9 Oct 2014 12:14:10 +0200 Subject: [PATCH] Refactoring JsonNode... --- srcs/Internals/JsonNode.cpp | 6 +- srcs/Internals/JsonNode.h | 141 +++++++++++++++++++++++++++--- srcs/JsonArray.cpp | 51 ++++++----- srcs/JsonArray.h | 5 +- srcs/JsonBuffer.cpp | 10 --- srcs/JsonBuffer.h | 11 ++- srcs/JsonContainer.cpp | 30 +++---- srcs/JsonContainer.h | 8 +- srcs/JsonObject.cpp | 35 ++++---- srcs/JsonValue.cpp | 120 ++++++------------------- srcs/JsonValue.h | 8 +- tests/JsonArray_PrintTo_Tests.cpp | 8 +- tests/JsonValueTests.cpp | 25 ++++-- 13 files changed, 250 insertions(+), 208 deletions(-) diff --git a/srcs/Internals/JsonNode.cpp b/srcs/Internals/JsonNode.cpp index f431c710..509def30 100644 --- a/srcs/Internals/JsonNode.cpp +++ b/srcs/Internals/JsonNode.cpp @@ -20,7 +20,7 @@ void JsonNode::writeTo(JsonWriter& writer) writer.writeString(content.asString); break; - case JSON_INTEGER: + case JSON_LONG: writer.writeInteger(content.asInteger); break; @@ -28,10 +28,6 @@ void JsonNode::writeTo(JsonWriter& writer) writer.writeBoolean(content.asBoolean); break; - case JSON_PROXY: - content.asProxy.target->writeTo(writer); - break; - default: // >= JSON_DOUBLE_0_DECIMALS writer.writeDouble(content.asDouble, type - JSON_DOUBLE_0_DECIMALS); break; diff --git a/srcs/Internals/JsonNode.h b/srcs/Internals/JsonNode.h index b11dac02..8eb048c9 100644 --- a/srcs/Internals/JsonNode.h +++ b/srcs/Internals/JsonNode.h @@ -5,14 +5,13 @@ class JsonBuffer; enum JsonNodeType { JSON_UNDEFINED, - JSON_PROXY, JSON_NULL, JSON_ARRAY, JSON_OBJECT, - JSON_KEY, + JSON_KEY_VALUE, JSON_BOOLEAN, JSON_STRING, - JSON_INTEGER, + JSON_LONG, JSON_DOUBLE_0_DECIMALS, JSON_DOUBLE_1_DECIMAL, JSON_DOUBLE_2_DECIMALS, @@ -23,10 +22,134 @@ class JsonWriter; struct JsonNode { - JsonNode* next; + JsonNode* next; JsonNodeType type; // <- TODO: hide - - void writeTo(JsonWriter&); + + void writeTo(JsonWriter&); // TODO: <- move in JsonNodeSerializer + + void setAsArray(JsonBuffer* buffer) + { + type = JSON_ARRAY; + content.asContainer.child = 0; + content.asContainer.buffer = buffer; + } + + void setAsBoolean(bool value) + { + type = JSON_BOOLEAN; + content.asBoolean = value; + } + + void setAsLong(int value) + { + type = JSON_LONG; + content.asInteger = value; + } + + void setAsString(char const* value) + { + type = JSON_STRING; + content.asString = value; + } + + void setAsDouble(double value, int decimals) + { + type = (JsonNodeType) (JSON_DOUBLE_0_DECIMALS + decimals); + content.asDouble = value; + } + + void setAsObject(JsonBuffer* buffer) + { + type = JSON_OBJECT; + content.asContainer.child = 0; + content.asContainer.buffer = buffer; + } + + void setAsObjectKeyValue(const char* key, JsonNode* value) + { + type = JSON_KEY_VALUE; + content.asKey.key = key; + content.asKey.value = value; + } + + bool getAsBoolean() + { + return type == JSON_BOOLEAN ? content.asBoolean : false; + } + + double getAsDouble() + { + return type > JSON_DOUBLE_0_DECIMALS ? content.asDouble : 0; + } + + long getAsInteger() + { + return type == JSON_LONG ? content.asInteger : 0; + } + + const char* getAsString() + { + return type == JSON_STRING ? content.asString : 0; + } + + JsonBuffer* getContainerBuffer() + { + return type == JSON_ARRAY || type == JSON_OBJECT ? content.asContainer.buffer : 0; + } + + JsonNode* getContainerChild() + { + return type == JSON_ARRAY || type == JSON_OBJECT ? content.asContainer.child : 0; + } + + const char* getAsObjectKey() + { + return type == JSON_KEY_VALUE ? content.asKey.key : 0; + } + + JsonNode* getAsObjectValue() + { + return type == JSON_KEY_VALUE ? content.asKey.value : 0; + } + + void addChildToContainer(JsonNode* childToAdd) + { + if (type != JSON_ARRAY && type != JSON_OBJECT) return; + + JsonNode* lastChild = content.asContainer.child; + + if (!lastChild) + { + content.asContainer.child = childToAdd; + return; + } + + while (lastChild->next) + lastChild = lastChild->next; + + lastChild->next = childToAdd; + } + + void removeChildFromContainer(JsonNode* childToRemove) + { + if (type != JSON_ARRAY && type != JSON_OBJECT) return; + + if (content.asContainer.child == childToRemove) + { + content.asContainer.child = childToRemove->next; + return; + } + + for (JsonNode* child = content.asContainer.child; child; child = child->next) + { + if (child->next == childToRemove) + child->next = childToRemove->next; + } + } + +private: + inline void writeArrayTo(JsonWriter&);// TODO: <- move in JsonNodeSerializer + inline void writeObjectTo(JsonWriter&);// TODO: <- move in JsonNodeSerializer union { @@ -35,7 +158,7 @@ struct JsonNode long asInteger; const char* asString; - struct + struct { const char* key; JsonNode* value; @@ -53,8 +176,4 @@ struct JsonNode } asProxy; } content; - -private: - inline void writeArrayTo(JsonWriter&); - inline void writeObjectTo(JsonWriter&); }; \ No newline at end of file diff --git a/srcs/JsonArray.cpp b/srcs/JsonArray.cpp index 85130037..cfa988bc 100644 --- a/srcs/JsonArray.cpp +++ b/srcs/JsonArray.cpp @@ -16,68 +16,71 @@ JsonValue JsonArray::operator[](int index) const void JsonArray::add(bool value) { - JsonNode* node = createNode(JSON_BOOLEAN); + JsonNode* node = createNode(); if (!node) return; - node->content.asBoolean = value; + node->setAsBoolean(value); addChild(node); } void JsonArray::add(char const* value) { - JsonNode* node = createNode(JSON_STRING); + JsonNode* node = createNode(); if (!node) return; - node->content.asString = value; + node->setAsString(value); addChild(node); } void JsonArray::add(double value, int decimals) { - JsonNode* node = createNode((JsonNodeType)(JSON_DOUBLE_0_DECIMALS + decimals)); + JsonNode* node = createNode(); if (!node) return; - node->content.asDouble = value; + node->setAsDouble(value, decimals); addChild(node); } void JsonArray::add(long value) { - JsonNode* node = createNode(JSON_INTEGER); + JsonNode* node = createNode(JSON_LONG); if (!node) return; - node->content.asInteger = value; + node->setAsLong(value); addChild(node); } -void JsonArray::add(JsonContainer innerContainer) +void JsonArray::add(JsonContainer nestedContainer) { - JsonNode* node = createNode(JSON_PROXY); + JsonNode* node = createNode(); if (!node) return; - node->content.asProxy.target = innerContainer._node; + *node = *nestedContainer._node; addChild(node); } JsonArray JsonArray::createNestedArray() { - JsonNode* node = createNestedContainer(JSON_ARRAY); + JsonNode* node = createNode(); + + if (node) + { + node->setAsArray(_node->getContainerBuffer()); + addChild(node); + } + return JsonArray(node); } JsonObject JsonArray::createNestedObject() { - JsonNode* node = createNestedContainer(JSON_OBJECT); + JsonNode* node = createNode(); + + if (node) + { + node->setAsObject(_node->getContainerBuffer()); + addChild(node); + } + return JsonObject(node); -} - -JsonNode* JsonArray::createNestedContainer(JsonNodeType type) -{ - JsonNode* node = createNode(type); - if (!node) return 0; - - node->content.asContainer.buffer = _node->content.asContainer.buffer; - addChild(node); - - return node; } \ No newline at end of file diff --git a/srcs/JsonArray.h b/srcs/JsonArray.h index 4891d2f9..2f64c9fe 100644 --- a/srcs/JsonArray.h +++ b/srcs/JsonArray.h @@ -21,12 +21,9 @@ public: void add(double value, int decimals=2); void add(int value) { add((long) value); } void add(long value); - void add(JsonContainer nestedArray); + void add(JsonContainer nestedArray); // TODO: should allow JsonValue too JsonArray createNestedArray(); JsonObject createNestedObject(); - -private: - JsonNode* createNestedContainer(JsonNodeType type); }; diff --git a/srcs/JsonBuffer.cpp b/srcs/JsonBuffer.cpp index 966c835a..050aaac4 100644 --- a/srcs/JsonBuffer.cpp +++ b/srcs/JsonBuffer.cpp @@ -20,14 +20,4 @@ JsonNode* JsonBuffer::createNode(JsonNodeType type) memset(node, 0, sizeof(JsonNode)); node->type = type; return node; -} - -JsonNode* JsonBuffer::createContainerNode(JsonNodeType type) -{ - JsonNode* node = createNode(type); - - if (node) - node->content.asContainer.buffer = this; - - return node; } \ No newline at end of file diff --git a/srcs/JsonBuffer.h b/srcs/JsonBuffer.h index 8a20d606..61339355 100644 --- a/srcs/JsonBuffer.h +++ b/srcs/JsonBuffer.h @@ -12,12 +12,16 @@ public: JsonArray createArray() { - return JsonArray(createContainerNode(JSON_ARRAY)); + JsonNode* node = createNode(); + if (node) node->setAsArray(this); + return JsonArray(node); } JsonObject createObject() { - return JsonObject(createContainerNode(JSON_OBJECT)); + JsonNode* node = createNode(); + if (node) node->setAsObject(this); + return JsonObject(node); } JsonValue createValue(); @@ -26,7 +30,6 @@ protected: virtual JsonNode* allocateNode() = 0; private: - JsonNode* createNode(JsonNodeType type); - JsonNode* createContainerNode(JsonNodeType type); + JsonNode* createNode(JsonNodeType type = JSON_UNDEFINED); }; diff --git a/srcs/JsonContainer.cpp b/srcs/JsonContainer.cpp index 1e2462f2..25d39902 100644 --- a/srcs/JsonContainer.cpp +++ b/srcs/JsonContainer.cpp @@ -37,7 +37,9 @@ JsonNode* JsonContainer::createNode(JsonNodeType type) { if (!_node) return 0; - JsonBuffer* buffer = _node->content.asContainer.buffer; + JsonBuffer* buffer = _node->getContainerBuffer(); + if (!buffer) return 0; + return buffer->createNode(type); } @@ -48,31 +50,19 @@ bool JsonContainer::checkNodeType(JsonNodeType expectedType) bool JsonContainer::operator==(const JsonContainer & other) const { - return _node == other._node; + return _node->getContainerChild() == other._node->getContainerChild(); } -void JsonContainer::addChild(JsonNode* newChild) +void JsonContainer::addChild(JsonNode* childToAdd) { - JsonNode* lastChild = _node->content.asContainer.child; - - if (!lastChild) - { - _node->content.asContainer.child = newChild = newChild; - return; - } - - while (lastChild->next) - lastChild = lastChild->next; - - lastChild->next = newChild; + if (_node) + _node->addChildToContainer(childToAdd); } -void JsonContainer::removeChildAfter(JsonNode* child, JsonNode* previous) +void JsonContainer::removeChild(JsonNode* childToRemove) { - if (previous) - previous->next = child->next; - else - _node->content.asContainer.child = child->next; + if (_node) + _node->removeChildFromContainer(childToRemove); } size_t JsonContainer::size() const diff --git a/srcs/JsonContainer.h b/srcs/JsonContainer.h index 6487a378..dc2a7279 100644 --- a/srcs/JsonContainer.h +++ b/srcs/JsonContainer.h @@ -43,7 +43,7 @@ protected: JsonNodeIterator beginChildren() const { - return JsonNodeIterator(_node ? _node->content.asContainer.child : 0); + return JsonNodeIterator(_node ? _node->getContainerChild() : 0); } JsonNodeIterator endChildren() const @@ -51,9 +51,9 @@ protected: return JsonNodeIterator(0); } - void addChild(JsonNode* newChild); - void removeChildAfter(JsonNode* child, JsonNode* previous); - JsonNode* createNode(JsonNodeType type); + void addChild(JsonNode*); + void removeChild(JsonNode*); + JsonNode* createNode(JsonNodeType type = JSON_UNDEFINED); bool checkNodeType(JsonNodeType expectedType); diff --git a/srcs/JsonObject.cpp b/srcs/JsonObject.cpp index e7fe07b1..83f5aaf9 100644 --- a/srcs/JsonObject.cpp +++ b/srcs/JsonObject.cpp @@ -18,30 +18,34 @@ JsonValue JsonObject::operator[](char const* key) void JsonObject::remove(char const* key) { - JsonNode* lastChild = 0; - for (JsonNodeIterator it = beginChildren(); it != endChildren(); ++it) { - const char* childKey = it->content.asKey.key; + const char* childKey = it->getAsObjectKey(); if (!strcmp(childKey, key)) { - removeChildAfter(*it, lastChild); - } - - lastChild = *it; + removeChild(*it); + } } } JsonArray JsonObject::createNestedArray(char const* key) { - JsonNode* node = createContainerNodeAt(key, JSON_ARRAY); + JsonNode* node = getOrCreateNodeAt(key); + + if (node) + node->setAsArray(_node->getContainerBuffer()); + return JsonArray(node); } JsonObject JsonObject::createNestedObject(char const* key) { - JsonNode* node = createContainerNodeAt(key, JSON_OBJECT); + JsonNode* node = getOrCreateNodeAt(key); + + if (node) + node->setAsObject(_node->getContainerBuffer()); + return JsonObject(node); } @@ -51,20 +55,19 @@ JsonNode* JsonObject::getOrCreateNodeAt(const char* key) for (JsonNodeIterator it = beginChildren(); it != endChildren(); ++it) { - const char* childKey = it->content.asKey.key; + const char* childKey = it->getAsObjectKey(); if (!strcmp(childKey, key)) - return it->content.asKey.value; + return it->getAsObjectValue(); } JsonNode* newValueNode = createNode(JSON_UNDEFINED); if (!newValueNode) return 0; - JsonNode* newKeyNode = createNode(JSON_KEY); + JsonNode* newKeyNode = createNode(JSON_KEY_VALUE); if (!newKeyNode) return 0; - newKeyNode->content.asKey.key = key; - newKeyNode->content.asKey.value = newValueNode; + newKeyNode->setAsObjectKeyValue(key, newValueNode); addChild(newKeyNode); @@ -76,9 +79,7 @@ JsonNode* JsonObject::createContainerNodeAt(char const* key, JsonNodeType type) JsonNode* node = getOrCreateNodeAt(key); if (!node) return 0; - node->type = type; - node->content.asContainer.child = 0; - node->content.asContainer.buffer = _node->content.asContainer.buffer; + node->setAsArray(_node->getContainerBuffer()); return node; } \ No newline at end of file diff --git a/srcs/JsonValue.cpp b/srcs/JsonValue.cpp index 9aa39080..fd784b96 100644 --- a/srcs/JsonValue.cpp +++ b/srcs/JsonValue.cpp @@ -6,144 +6,80 @@ void JsonValue::operator=(bool value) { - if (!_node) return; - - _node->type = JSON_BOOLEAN; - _node->content.asBoolean = value; + if (_node) + _node->setAsBoolean(value); } void JsonValue::operator=(char const* value) { - if (!_node) return; - - _node->type = JSON_STRING; - _node->content.asString = value; -} - -void JsonValue::operator=(double value) -{ - set(value, 2); + if (_node) + _node->setAsString(value); } void JsonValue::set(double value, int decimals) { - if (!_node) return; - - _node->type = (JsonNodeType) (JSON_DOUBLE_0_DECIMALS + decimals); - _node->content.asDouble = value; + if (_node) + _node->setAsDouble(value, decimals); } void JsonValue::operator=(int value) { - if (!_node) return; - - _node->type = JSON_INTEGER; - _node->content.asInteger = value; + if (_node) + _node->setAsLong(value); } +// TODO: it's a duplicate void JsonValue::operator=(const JsonContainer& object) { - setAsProxyTo(object._node); + if (!_node) + { + _node = object._node; + } + else + { + *_node = *object._node; + } } +// TODO: it's a duplicate void JsonValue::operator=(JsonValue const& value) { if (!_node) { - _node = value._node; - return; + _node = value._node; } - - JsonNodeType nodeType = value._node ? value._node->type : JSON_UNDEFINED; - - switch (nodeType) + else { - case JSON_UNDEFINED: - _node->type = JSON_UNDEFINED; - break; - - case JSON_INTEGER: - _node->type = JSON_INTEGER; - _node->content.asInteger = value._node->content.asInteger; - break; - - case JSON_DOUBLE_0_DECIMALS: - - case JSON_OBJECT: - case JSON_ARRAY: - case JSON_PROXY: - setAsProxyTo(value._node); - break; - - default: *_node = *value._node; - break; - } + } } JsonValue::operator bool() const { - const JsonNode* node = getActualNode(); - - if (!node || node->type != JSON_BOOLEAN) return 0; - - return node->content.asBoolean; + return _node ? _node->getAsBoolean() : false; } JsonValue::operator char const*() const { - const JsonNode* node = getActualNode(); - - if (!node || node->type != JSON_STRING) return 0; - - return node->content.asString; + return _node ? _node->getAsString() : 0; } JsonValue::operator double() const { - const JsonNode* node = getActualNode(); - - if (!node || node->type < JSON_DOUBLE_0_DECIMALS) return 0; - - return node->content.asDouble; + return _node ? _node->getAsDouble() : 0; } -JsonValue::operator int() const +JsonValue::operator long() const { - const JsonNode* node = getActualNode(); - - if (!node || node->type != JSON_INTEGER) return 0; - - return node->content.asInteger; + return _node ? _node->getAsInteger() : 0; } JsonValue::operator JsonArray() const { - return JsonArray(getActualNode()); + return JsonArray(_node); } JsonValue::operator JsonObject() const { - return JsonObject(getActualNode()); -} - -void JsonValue::setAsProxyTo(JsonNode* target) -{ - if (_node) - { - _node->type = JSON_PROXY; - _node->content.asProxy.target = target; - } - - _node = target; -} - -JsonNode* JsonValue::getActualNode() const -{ - JsonNode* target = _node; - - while (target && target->type == JSON_PROXY) - target = target->content.asProxy.target; - - return target; + return JsonObject(_node); } \ No newline at end of file diff --git a/srcs/JsonValue.h b/srcs/JsonValue.h index 400574b5..7d3c44a9 100644 --- a/srcs/JsonValue.h +++ b/srcs/JsonValue.h @@ -21,7 +21,7 @@ public: void operator=(bool); void operator=(const char*); - void operator=(double); + void operator=(double x) { set(x, 2); } void operator=(int); void operator=(const JsonContainer&); void operator=(const JsonValue&); @@ -29,7 +29,8 @@ public: operator bool() const; operator const char*() const; operator double() const; - operator int() const; + operator long() const; + operator int() const { return operator long(); } operator JsonArray() const; operator JsonObject() const; @@ -37,7 +38,4 @@ public: private: JsonNode* _node; - - void setAsProxyTo(JsonNode*); - JsonNode* getActualNode() const; }; \ No newline at end of file diff --git a/tests/JsonArray_PrintTo_Tests.cpp b/tests/JsonArray_PrintTo_Tests.cpp index 71246ff3..3f22fc7b 100644 --- a/tests/JsonArray_PrintTo_Tests.cpp +++ b/tests/JsonArray_PrintTo_Tests.cpp @@ -135,18 +135,14 @@ TEST_F(JsonArray_PrintTo_Tests, OneBooleanOverCapacity) TEST_F(JsonArray_PrintTo_Tests, OneEmptyNestedArray) { - JsonArray nestedArray = json.createArray(); - - array.add(nestedArray); + array.createNestedArray(); outputMustBe("[[]]"); } TEST_F(JsonArray_PrintTo_Tests, OneEmptyNestedHash) { - JsonObject nestedObject = json.createObject(); - - array.add(nestedObject); + array.createNestedObject(); outputMustBe("[{}]"); } \ No newline at end of file diff --git a/tests/JsonValueTests.cpp b/tests/JsonValueTests.cpp index 4d92b142..80fc99fd 100644 --- a/tests/JsonValueTests.cpp +++ b/tests/JsonValueTests.cpp @@ -59,7 +59,7 @@ TEST_F(JsonValueTests, CanStoreObject) EXPECT_EQ(innerObject1, (JsonObject) jsonValue1); } -TEST_F(JsonValueTests, IntegerValuesAreCopied) +TEST_F(JsonValueTests, IntegersAreCopiedByValue) { jsonValue1 = 123; jsonValue2 = jsonValue1; @@ -68,7 +68,7 @@ TEST_F(JsonValueTests, IntegerValuesAreCopied) EXPECT_EQ(123, (int) jsonValue2); } -TEST_F(JsonValueTests, DoubleValuesAreCopied) +TEST_F(JsonValueTests, DoublesAreCopiedByValue) { jsonValue1 = 123.45; jsonValue2 = jsonValue1; @@ -77,7 +77,7 @@ TEST_F(JsonValueTests, DoubleValuesAreCopied) EXPECT_EQ(123.45, (double) jsonValue2); } -TEST_F(JsonValueTests, BooleanValuesAreCopied) +TEST_F(JsonValueTests, BooleansAreCopiedByValue) { jsonValue1 = true; jsonValue2 = jsonValue1; @@ -86,7 +86,7 @@ TEST_F(JsonValueTests, BooleanValuesAreCopied) EXPECT_TRUE((bool) jsonValue2); } -TEST_F(JsonValueTests, CharPointersAreCopied) +TEST_F(JsonValueTests, StringsAreCopiedByValue) { jsonValue1 = "hello"; jsonValue2 = jsonValue1; @@ -95,14 +95,27 @@ TEST_F(JsonValueTests, CharPointersAreCopied) EXPECT_STREQ("hello", (const char*) jsonValue2); } -TEST_F(JsonValueTests, ObjectPointsAreCopied) + +TEST_F(JsonValueTests, ObjectsAreCopiedByReference) { JsonObject object = json.createObject(); jsonValue1 = object; jsonValue2 = jsonValue1; - + object["hello"] = "world"; + EXPECT_EQ(1, ((JsonObject) jsonValue2).size()); +} + +TEST_F(JsonValueTests, ArraysAreCopiedByReference) +{ + JsonArray array = json.createArray(); + + jsonValue1 = array; + jsonValue2 = jsonValue1; + + array.add("world"); + EXPECT_EQ(1, ((JsonObject) jsonValue2).size()); } \ No newline at end of file