Lacks functions for removing elements in JSON array. about rapidjson HOT 10 CLOSED

Sounds very useful, 👍 from my side.

Some remarks from my side:

• In order to avoid surprises (compared to std::vector and the other C++ containers), the implementation should preserve the order of the remaining values in the array.
• A similar iterator-based Erase function should be added for MemberIterators (again, preferably order-preserving).

Last, but not least, adding an operator== to GenericValue enables the C++ erase-remove idiom, std::remove_if works already:

  // remove all 'null' values
  bool IsNull( const Value& v ) { return v.IsNull(); }
  d.Erase(std::remove_if(d.Begin(), d.End(), IsNull), d.End());

The RemoveMember function currently shuffles the order (for performance reasons), which would be inconsistent. In practice, moving all elements should be quite fast as the memory is contiguous. But we could of course also add a flag (similar to kParseFlag) for these operations. What do you think?

from rapidjson.

I added operator==() in 71c8402.

For RemoveMember(), I think it is OK to change the order as JSON object can be thought as an unordered map. But it is also possible to add a function like EraseMember() which preserve order as JSON array.

from rapidjson.

In #92, I've added an Erase overload, taking MemberIterators. Would you prefer to call it EraseMember? The distinction based on the iterator type should be sufficient, isn't it?

from rapidjson.

I think that will be easier to identify which functions are for object and which are for array.

from rapidjson.

Ok, added to #92 in fc6d7a5.

from rapidjson.

I will amend the documentation before merging this branch.

from rapidjson.

Sure, thanks.

from rapidjson.

I try to add erase-remove in unit tests at b52e078. It passed gcc but not VC2013:

1>C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\include\algorithm(1735): error C2248: 'rapidjson::GenericValue<rapidjson::UTF8<char>,rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>>::GenericValue' : cannot access private member declared in class 'rapidjson::GenericValue<rapidjson::UTF8<char>,rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>>'
1>          ..\..\include\rapidjson/document.h(357) : see declaration of 'rapidjson::GenericValue<rapidjson::UTF8<char>,rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>>::GenericValue'
1>          C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\include\algorithm(1748) : see reference to function template instantiation '_FwdIt std::_Remove<_Iter,_Ty>(_FwdIt,_FwdIt,const _Ty &)' being compiled
1>          with
1>          [
1>              _FwdIt=rapidjson::GenericValue<rapidjson::UTF8<char>,rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>> *
1>  ,            _Iter=rapidjson::GenericValue<rapidjson::UTF8<char>,rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>> *
1>  ,            _Ty=rapidjson::GenericValue<rapidjson::UTF8<char>,rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>>
1>          ]
1>          ..\..\test\unittest\valuetest.cpp(688) : see reference to function template instantiation '_FwdIt std::remove<rapidjson::GenericValue<rapidjson::UTF8<char>,rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>>*,rapidjson::GenericValue<rapidjson::UTF8<char>,rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>>>(_FwdIt,_FwdIt,const _Ty &)' being compiled
1>          with
1>          [
1>              _FwdIt=rapidjson::GenericValue<rapidjson::UTF8<char>,rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>> *
1>  ,            _Ty=rapidjson::GenericValue<rapidjson::UTF8<char>,rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>>
1>          ]

I think it is because VC2013 force to use C++11 and remove() requires MoveAssignable.

from rapidjson.

Mmh, it's probably due to GenericValue's requirement to take a non-const reference to its public assignment operator and in C++11, an explicit std::move is performed before the assignment. It fails with a similar error on GCC 4.9 with -std=c++11.

We might need to add both a MoveConstructor and and MoveAssignment operator in C++11 mode. A very simple approach (hidden behind a macro RAPIDJSON_CXX11) is available in pah/rapidjson-upstream@8005b55a.

With this (and the signedness fix in my issue91removeelement branch, the "erase-remove" test works both on GCC 4.9 in C++11 mode and VS 2013 (when RAPIDJSON_CXX11 is manually defined).

from rapidjson.

Side note: With the move constructor, the Move function is no longer needed in C++11 code.

from rapidjson.