jsoncons
jsoncons copied to clipboard
Does returning a proxy make basic_json::operator[] too complicated?
From the beginnings of this library in 2013, influenced by the proxy idiom described in Scott Meyers' book Effective Modern C++, the non const basic_json::operator[](const string_view_type&)
returns a proxy instead of a basic_json&
,
proxy_type operator[](const string_view_type& key)
The proxy allows the implementation to distinguish between assignment, e.g.
json j;
j["foo"] = 1;
and access, e.g.
json j;
json val = j["foo"];
and throw an exception if attempting to access "foo" if "foo" is not present, similar to Python dictionary. That seemed to me in 2013 to be a good feature to have. This behavior is the same as that currently defined for the const overload
const basic_json& operator[](const string_view_type& key) const
(where assignment is an error, so no need for a proxy).
However, there are some problems.
- Proxies don't work well with auto.
-
auto& val = j[0]
is okay with a json array and returns a json reference as expected, butauto& val = j["foo"]
is not okay with a json object because it returns a reference to a temporary proxy type. This will result in a compile time warning, for the temporary, but not an error. Instead it needs to be written explicitly asjson& val = j["foo"]
. -
auto val = j["foo"]
will result in a compile time error, because the proxy copy and move constructors and assignment operators have been made private. But having private proxy assignment operators causes the issue discussed in #312. - There is a 2017 proposal P0672R0 to solve the problem of proxies and auto, but it doesn't seem to have much traction.
- The implementation of the proxy class is currently about 1000 lines of code, including deprecated functions.
- It's different from the behavior of
std::map<Key,T>
, which will insert(key,T())
ifkey
does not exist.std::map<Key,T>
doesn't have a const overload ofoperator[]
. Instead it has throwing const and non-const overloads ofat
, which may be used for access or replacement if the key already exists (jsoncons also supportsat
.) - It's different from other json libraries, which stay consistent with
std::map<Key,T>
for the non-const overload ofoperator[]
.
For these reasons, I'm considering removing the proxy, i.e. removing 1000 lines of code and changing to
basic_json& operator[](const string_view_type& key)
where, when used as an accessor, operator[]
will insert (key,json())
if key
does not exist. Note that json()
is an empty object.
In terms of impact, there should be no impact for code on the "good path". In the absence of errors, all code should work as before. I've ran the tests with this change on a branch, and all tests pass except tests that are testing behavior with keys that don't exist.
But in the case of attempting to access a key that does not exist, the behavior will change from throwing to inserting (key,json())
.
No change is proposed for the const overload operator[](const string_view_type& key) const
. It will continue to throw.
Comments welcome.