erase multiple elements from a json object

[best9541]

Bug Report

• What is the issue you have?
  I want to erase several elements of a json object having a specific criteria.
  Therefore I use json::erase with std::remove_if and a lambda function:

TEST(Parse_Sd_service_timing_test, remove_optional)
{
    json j = {{"val1", nullptr}, {"val2", 2}, {"val3", nullptr}, {"val4", 4}};
    std::cout << j.dump(2) << std::endl << std::endl;

    j.erase(std::remove_if(j.begin(), j.end(), [](json const& j2) { return j2.is_null(); }),
            j.end());

    std::cout << j.dump(2) << std::endl << std::endl;
}

The first output is

{
"val1": null,
"val2": 2,
"val3": null,
"val4": 4
}

After removal of optional elements (null), the output is

{
"val1": 2,
"val2": 4
}

It seems that only the values having 'null' are deleted, but not the associating key.
I would have expected that the key & value get's deleted to get the following output:

{
"val2": 2,
"val4": 4
}

[gregmarr]

You can't use remove_if on a map, which is what you are trying to do:

http://en.cppreference.com/w/cpp/algorithm/remove

These algorithms cannot be used with associative containers such as std::set and std::map because ForwardIt does not dereference to a MoveAssignable type (the keys in these containers are not modifiable)

The fact that it even compiles is a bit odd. There must be something about the extra stuff in the class for it being multiple types at once that makes it compile.

[gregmarr]

Ah, I think this is because the raw begin() and end() return iterators which make the object look like an array of values. The iterator_wrapper() would make it look like a map, and would then probably fail to compile.

[nlohmann]

I agree with @gregmarr that there seems to be no way to achieve this with remove_if. Using iterator_wrapper seems not to be an option either, because it is designed to work with range-based for loops, and in a remove_if you cannot use json::iteration_proxy<iterator> as parameter type for the lambda, because it is private.

[nlohmann]

Good news: you can use the solution from https://stackoverflow.com/a/800984/266378:

    auto iter = j.begin();
    for(; iter != j.end(); ) {
        if (iter.value().is_null()) {
            j.erase(iter++);
        } else {
            ++iter;
        }
    }

[gregmarr]

I think this slight change would be more idiomatic, and potentially safer.

auto iter = j.begin();
for(; iter != j.end(); ) {
    if (iter.value().is_null()) {
        iter = j.erase(iter);
    } else {
        ++iter;
    }
}

[best9541]

Thx. It is not so obvious that it's implemented as map. Then it's clean it will not work with remove_if.

[pboettch]

@best9541 no, the iter++ is placed correctly. erase() returns the next iterator in a safe manner and erases the element in the container.

[nlohmann]

@gregmarr Right. This also avoids a few copies of the iterator.

@best9541 Can I close this issue?

[best9541]

yes.
Btw: We can even use const_iterator as we don't change the iterator

auto iter = j.cbegin();
    for (; iter != j.cend();) {
        if (iter.value().is_null()) {
            iter = j.erase(iter);
        }
        else {
            ++iter;
        }
    }

[alexgeek]

Anyway to use the new std::erase_if (c++20) for this? 🤔

Oh I see not quite like that, but you can use .erase and std::remove_if. (https://github.com/nlohmann/json/pull/3109/files#diff-009389a614bbd69629b49cdbc3861bb75a6d59508301270009d338d2c2772cfcR759)

[falbrechtskirchinger]

This should work but I haven't tested it:

std::erase_if(j.get_ref<json::object_t &>(), pred);

[alexgeek]

This should work but I haven't tested it:

std::erase_if(j.get_ref<json::object_t &>(), pred);

Thanks, for me turns out I actually wanted .get_ref<json::array_t&>() but got it working 👍

[RotsiserMho]

Hmm, I'm having trouble getting the proposed solution to work. According to the docs, iterators after the one that's erased are invalidated so erasing in a loop of any kind seems impossible.

[falbrechtskirchinger]

Hmm, I'm having trouble getting the proposed solution to work. According to the docs, iterators after the one that's erased are invalidated so erasing in a loop of any kind seems impossible.

The proposed solution avoids that problem. Why is that not working for you?