overload of at() with default value

[dariomt]

My use case is:

• use find() to look for a key
• if the key is not found return a default_value of type T (possibly moved in and out)
• if the key is found return the object converted to type T (only if T is compatible)

e.g.

template <typename T>
T at (const typename object_t::key_type & key, T default_value) const noexcept
{
    auto it = find(key);
    if (it != end()) return *it;
    return default_value;
}

What do you think?

[nlohmann]

I don't think this is a good idea, because I would like the operator[] to "feel" like the one of std::map. And there, an exception is thrown if the container does not have an element with the specified key.

[dariomt]

OK, understood.
Would you consider adding this functionality with a different name?

[rigtorp]

Something equivalent of this would be great. I don't know why the STL doesn't have this. In C++17 there is insert_or_assign(), but no equivalent to python's dict.get() https://docs.python.org/2/library/stdtypes.html#dict.get. JsonCpp have this too http://open-source-parsers.github.io/jsoncpp-docs/doxygen/class_json_1_1_value.html#a28282c9b76fa031eba7a1843c47c16fe
I don't know why the STL missed this common usecase.

[nlohmann]

I'm on holiday right now. I'll have a look then.

[nlohmann]

I think the insert_or_assign() would be misleading, because as far as I understood @dariomt, the object shall be left unchanged.

Any name proposals better than at?

[rigtorp]

Ah, insert_or_assign is different than want @dariomt is asking. I'm just suggesting to add insert_or_assign, it's added to the container types in c++17. I think at() is good name for what @dariomt wants and similar to the STL. Maybe at_default() is better? boost::property_tree might give some inspiration: http://www.boost.org/doc/libs/1_59_0/doc/html/property_tree/accessing.html

[dariomt]

maybe at_or_default()?

[mbunkus]

The standard containers' at() function returns a reference to the member allowing modification. If our new function returns a copy instead (which it has to do in order to be able to return the default value) then I'd rather the function wasn't named at() in order to avoid ambiguity.

Qt's QList template class provides such a function called value() instead. It also provides an at() compatible with the standard library's at() thereby completely avoiding ambiguity. QList provides two variants of value(), one taking only a single index argument which returns a default-constructed instance in case of non-existence and an overload taking both an index and the default value to return. I rather like this flexibility.

[nlohmann]

Good point with the reference! I guess value() would be a good candidate. I see if I can fix something this weekend.

[nlohmann]

What do you think of the following:

value_type value(const typename object_t::key_type& key, value_type default_value = basic_json()) const
{
    // at only works for objects
    if (m_type != value_t::object)
    {
        throw std::domain_error("cannot use value() with " + type_name());
    }

    // if key is found, return value; otherwise, return given default value
    auto it = find(key);
    if (it != end())
    {
        return *it;
    }
    else
    {
        return default_value;
    }
}

It allows code like int i = j.value("key_key", 17); or j.value("other_key") to use null as default value.

[nlohmann]

I forgot: Of course, the above could could be noexcept and also return the default value if called on a JSON value other than an object.

[mbunkus]

I like your proposed implementation.

About returning the default value instead of throwing: from the perspective of least surprise I'd argue that value() only makes sense for array and object types. Therefore it should not work for other types.

Note that it's also easier to disallow other types first and to change the API later if users request it to work on other types as well then the other way around: if you allow it now and decide to disallow it later it would break the API and therefore potentially existing applications.

[dariomt]

As I mentioned in my original post, my use case is more in line with:

template <typename T>
T value(const typename object_t::key_type& key, T default_value) const

because when I'm looking for a key I already know the type the associated value should have, and I expect that I can get the converted-to-T value if the key is found, or the default-T if it is not found.

To me that's the meaning value() conveys.

[nlohmann]

@dariomt

I tried, but with the template I fail to compile a call like j.value("key", "default string"), because of

error: no viable conversion from 'const value_type' (aka 'const nlohmann::basic_json<std::map, std::vector, std::__1::basic_string<char>, bool, long long, double, std::allocator>') to 'const char *'

When I write j.value("key", std::string("default string")), it works.

Any idea?

[dariomt]

My guess:
The return statement inside value() tries to convert from basic_json to T, but such conversion is not possible because basic_json is implicitly convertible to std::string but not to const char*. That's why the second case works, because in that case T is std::string.

My suggestion: add an overload to take care of that case, that creates a StringType for the default value and redirects to the main value().

// overload for a default value of type const char*
StringType value(const typename object_t::key_type& key, const char* default_value) const
{
    return value(key, StringType(default_value));
}

If you don't like my suggestion, then you could static_assert that basic_json is implicitly convertible to T inside value(), to give a user-friendly error message at compile time.

[gregmarr]

If using the template, you need to make it deduce a type that is implicitly convertible both to and from value_type. This means that either you provide the T explicitly as in j.valuestd::string("key", "default string"), or you make sure that the default value is that type, as in j.value("key", "default string"s). To put the error at the point of use, you could add enable_if as you do in the constructors.

[nlohmann]

I added the function with some documentation and example: http://nlohmann.github.io/json/classnlohmann_1_1basic__json_a26b7e4876af25eae5800ce43f93aaef2.html#a26b7e4876af25eae5800ce43f93aaef2

[nlohmann]

Hi there, one more question: What do you think of a second version of value which always returns a basic_json value and implicitly return a null value as default?

value_type value(const typename object_t::key_type &key)

[gregmarr]

I think this "with default" version would be much more useful now that [] can no longer be used on const objects. I did a local build of our code with the updated version, and had to do a lot of replacing of foo[key] with *foo.find(key), and would much prefer foo.value(key).

[nlohmann]

Ok, will so.

[nlohmann]

Another idea for the version without explicit default value: Instead of returning json() as default value, I could return *this. This would allow a version of value(key) that always returns const references.

[nlohmann]

After thinking about it for quite a while, removing the const version of operator[] was a mistake. I re-added it in a previous commit. I think now the value function does not need another version with implicit default value, right?