From 8c6f64c11163af9c6847b697f83e1483721e2373 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Wed, 12 Apr 2017 21:00:13 +0200 Subject: [PATCH] Added `JsonArray::remove(iterator)` and `JsonObject::remove(iterator)` (issue #479) --- CHANGELOG.md | 6 +++ include/ArduinoJson/Data/List.hpp | 33 ++++++------ .../ArduinoJson/Data/ListConstIterator.hpp | 8 +++ include/ArduinoJson/Data/ListIterator.hpp | 13 +++++ include/ArduinoJson/JsonArray.hpp | 34 +++++-------- include/ArduinoJson/JsonObject.hpp | 50 +++++++++++-------- test/JsonArray/removeAt.cpp | 36 +++++++++++-- test/JsonObject/remove.cpp | 13 +++++ 8 files changed, 130 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9673cd8..7fade029 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ ArduinoJson: change log ======================= +HEAD +---- + +* Added `JsonArray::remove(iterator)` (issue #479) +* Added `JsonObject::remove(iterator)` + v5.8.4 ------ diff --git a/include/ArduinoJson/Data/List.hpp b/include/ArduinoJson/Data/List.hpp index b1f9d79f..2eb732c3 100644 --- a/include/ArduinoJson/Data/List.hpp +++ b/include/ArduinoJson/Data/List.hpp @@ -48,6 +48,20 @@ class List { return nodeCount; } + iterator add() { + node_type *newNode = new (_buffer) node_type(); + + if (_firstNode) { + node_type *lastNode = _firstNode; + while (lastNode->next) lastNode = lastNode->next; + lastNode->next = newNode; + } else { + _firstNode = newNode; + } + + return iterator(newNode); + } + iterator begin() { return iterator(_firstNode); } @@ -62,22 +76,8 @@ class List { return const_iterator(NULL); } - protected: - node_type *addNewNode() { - node_type *newNode = new (_buffer) node_type(); - - if (_firstNode) { - node_type *lastNode = _firstNode; - while (lastNode->next) lastNode = lastNode->next; - lastNode->next = newNode; - } else { - _firstNode = newNode; - } - - return newNode; - } - - void removeNode(node_type *nodeToRemove) { + void remove(iterator it) { + node_type *nodeToRemove = it._node; if (!nodeToRemove) return; if (nodeToRemove == _firstNode) { _firstNode = nodeToRemove->next; @@ -87,6 +87,7 @@ class List { } } + protected: JsonBuffer *_buffer; node_type *_firstNode; }; diff --git a/include/ArduinoJson/Data/ListConstIterator.hpp b/include/ArduinoJson/Data/ListConstIterator.hpp index 6cda88f1..bce1bfa5 100644 --- a/include/ArduinoJson/Data/ListConstIterator.hpp +++ b/include/ArduinoJson/Data/ListConstIterator.hpp @@ -38,6 +38,14 @@ class ListConstIterator { return *this; } + ListConstIterator &operator+=(size_t distance) { + while (_node && distance) { + _node = _node->next; + --distance; + } + return *this; + } + private: const ListNode *_node; }; diff --git a/include/ArduinoJson/Data/ListIterator.hpp b/include/ArduinoJson/Data/ListIterator.hpp index f703fcca..a491866f 100644 --- a/include/ArduinoJson/Data/ListIterator.hpp +++ b/include/ArduinoJson/Data/ListIterator.hpp @@ -13,9 +13,14 @@ namespace ArduinoJson { namespace Internals { +template +class List; + // A read-write forward iterator for List template class ListIterator { + friend class List; + public: explicit ListIterator(ListNode *node = NULL) : _node(node) {} @@ -39,6 +44,14 @@ class ListIterator { return *this; } + ListIterator &operator+=(size_t distance) { + while (_node && distance) { + _node = _node->next; + --distance; + } + return *this; + } + operator ListConstIterator() const { return ListConstIterator(_node); } diff --git a/include/ArduinoJson/JsonArray.hpp b/include/ArduinoJson/JsonArray.hpp index 40175acc..6881520c 100644 --- a/include/ArduinoJson/JsonArray.hpp +++ b/include/ArduinoJson/JsonArray.hpp @@ -111,16 +111,15 @@ class JsonArray : public Internals::JsonPrintable, // Gets the value at the specified index. template typename Internals::JsonVariantAs::type get(size_t index) const { - node_type *node = findNode(index); - return node ? node->content.as() - : Internals::JsonVariantDefault::get(); + const_iterator it = begin() += index; + return it != end() ? it->as() : Internals::JsonVariantDefault::get(); } // Check the type of the value at specified index. template bool is(size_t index) const { - node_type *node = findNode(index); - return node ? node->content.is() : false; + const_iterator it = begin() += index; + return it != end() ? it->is() : false; } // Creates a JsonArray and adds a reference at the end of the array. @@ -133,8 +132,9 @@ class JsonArray : public Internals::JsonPrintable, // Removes element at specified index. void removeAt(size_t index) { - removeNode(findNode(index)); + remove(begin() += index); } + using Internals::List::remove; // Returns a reference an invalid JsonArray. // This object is meant to replace a NULL pointer. @@ -198,28 +198,18 @@ class JsonArray : public Internals::JsonPrintable, } private: - node_type *findNode(size_t index) const { - node_type *node = _firstNode; - while (node && index--) node = node->next; - return node; - } - template bool set_impl(size_t index, TValueRef value) { - node_type *node = findNode(index); - if (!node) return false; - - return Internals::ValueSetter::set(_buffer, node->content, - value); + iterator it = begin() += index; + if (it == end()) return false; + return Internals::ValueSetter::set(_buffer, *it, value); } template bool add_impl(TValueRef value) { - node_type *node = addNewNode(); - if (!node) return false; - - return Internals::ValueSetter::set(_buffer, node->content, - value); + iterator it = Internals::List::add(); + if (it == end()) return false; + return Internals::ValueSetter::set(_buffer, *it, value); } }; diff --git a/include/ArduinoJson/JsonObject.hpp b/include/ArduinoJson/JsonObject.hpp index b55e9601..c057b71e 100644 --- a/include/ArduinoJson/JsonObject.hpp +++ b/include/ArduinoJson/JsonObject.hpp @@ -247,14 +247,14 @@ class JsonObject : public Internals::JsonPrintable, typename TypeTraits::EnableIf::value, bool>::type containsKey(const TString& key) const { - return findNode(key) != NULL; + return findKey(key) != end(); } // // bool containsKey(TKey); // TKey = const char*, const char[N], const FlashStringHelper* template bool containsKey(const TString* key) const { - return findNode(key) != NULL; + return findKey(key) != end(); } // Removes the specified key and the associated value. @@ -265,15 +265,18 @@ class JsonObject : public Internals::JsonPrintable, typename TypeTraits::EnableIf::value, void>::type remove(const TString& key) { - removeNode(findNode(key)); + remove(findKey(key)); } // // void remove(TKey); // TKey = const char*, const char[N], const FlashStringHelper* template void remove(const TString* key) { - removeNode(findNode(key)); + remove(findKey(key)); } + // + // void remove(iterator) + using Internals::List::remove; // Returns a reference an invalid JsonObject. // This object is meant to replace a NULL pointer. @@ -286,41 +289,44 @@ class JsonObject : public Internals::JsonPrintable, private: // Returns the list node that matches the specified key. template - node_type* findNode(TStringRef key) const { - for (node_type* node = _firstNode; node; node = node->next) { - if (Internals::StringTraits::equals(key, node->content.key)) - return node; + iterator findKey(TStringRef key) { + iterator it; + for (it = begin(); it != end(); ++it) { + if (Internals::StringTraits::equals(key, it->key)) break; } - return NULL; + return it; + } + template + const_iterator findKey(TStringRef key) const { + return const_cast(this)->findKey(key); } template typename Internals::JsonVariantAs::type get_impl( TStringRef key) const { - node_type* node = findNode(key); - return node ? node->content.value.as() - : Internals::JsonVariantDefault::get(); + const_iterator it = findKey(key); + return it != end() ? it->value.as() + : Internals::JsonVariantDefault::get(); } template bool set_impl(TStringRef key, TValueRef value) { - node_type* node = findNode(key); - if (!node) { - node = addNewNode(); - if (!node) return false; + iterator it = findKey(key); + if (it == end()) { + it = Internals::List::add(); + if (it == end()) return false; - bool key_ok = Internals::ValueSetter::set( - _buffer, node->content.key, key); + bool key_ok = + Internals::ValueSetter::set(_buffer, it->key, key); if (!key_ok) return false; } - return Internals::ValueSetter::set(_buffer, node->content.value, - value); + return Internals::ValueSetter::set(_buffer, it->value, value); } template bool is_impl(TStringRef key) const { - node_type* node = findNode(key); - return node ? node->content.value.is() : false; + const_iterator it = findKey(key); + return it != end() ? it->value.is() : false; } template diff --git a/test/JsonArray/removeAt.cpp b/test/JsonArray/removeAt.cpp index 4f233610..c85f75e0 100644 --- a/test/JsonArray/removeAt.cpp +++ b/test/JsonArray/removeAt.cpp @@ -22,7 +22,7 @@ class JsonArray_Remove_Tests : public ::testing::Test { #define TEST_(name) TEST_F(JsonArray_Remove_Tests, name) -TEST_(RemoveFirstElement) { +TEST_(RemoveFirstByIndex) { _array.removeAt(0); EXPECT_EQ(2, _array.size()); @@ -30,7 +30,7 @@ TEST_(RemoveFirstElement) { EXPECT_STREQ("three", _array[1]); } -TEST_(RemoveMiddleElement) { +TEST_(RemoveMiddleByIndex) { _array.removeAt(1); EXPECT_EQ(2, _array.size()); @@ -38,10 +38,40 @@ TEST_(RemoveMiddleElement) { EXPECT_STREQ("three", _array[1]); } -TEST_(RemoveLastElement) { +TEST_(RemoveLastByIndex) { _array.removeAt(2); EXPECT_EQ(2, _array.size()); EXPECT_STREQ("one", _array[0]); EXPECT_STREQ("two", _array[1]); } + +TEST_(RemoveFirstByIterator) { + JsonArray::iterator it = _array.begin(); + _array.remove(it); + + EXPECT_EQ(2, _array.size()); + EXPECT_STREQ("two", _array[0]); + EXPECT_STREQ("three", _array[1]); +} + +TEST_(RemoveMiddleByIterator) { + JsonArray::iterator it = _array.begin(); + ++it; + _array.remove(it); + + EXPECT_EQ(2, _array.size()); + EXPECT_STREQ("one", _array[0]); + EXPECT_STREQ("three", _array[1]); +} + +TEST_(RemoveLastByIterator) { + JsonArray::iterator it = _array.begin(); + ++it; + ++it; + _array.remove(it); + + EXPECT_EQ(2, _array.size()); + EXPECT_STREQ("one", _array[0]); + EXPECT_STREQ("two", _array[1]); +} diff --git a/test/JsonObject/remove.cpp b/test/JsonObject/remove.cpp index 8018addd..595194d8 100644 --- a/test/JsonObject/remove.cpp +++ b/test/JsonObject/remove.cpp @@ -29,3 +29,16 @@ TEST_(SizeUntouched_WhenRemoveIsCalledWithAWrongKey) { EXPECT_EQ(1, _object.size()); } + +TEST_(RemoveByIterator) { + DynamicJsonBuffer _jsonBuffer; + JsonObject& _object = _jsonBuffer.parseObject("{\"a\":0,\"b\":1,\"c\":2}"); + + for (JsonObject::iterator it = _object.begin(); it != _object.end(); ++it) { + if (it->value == 1) _object.remove(it); + } + + char result[64]; + _object.printTo(result); + EXPECT_STREQ("{\"a\":0,\"c\":2}", result); +}