json: Do not throw exception when default_value's type does not match the actual type

Thanks for this great library. I have been quite happy about almost every aspect of the library, except one point about the value method on object.

Consider this sample code.

int main() {
  nlohmann::json j = { {"b", nullptr}};
  std::string result = j.value("b", "1"); // std::domain_error: type must be string, but is null
  return 0;
}

When I’m using value to specify a default value of a key, I pretty much don’t care about whether it exists or not, whether it is a valid value or not. It’s pretty much like, if it failed to fetch a value of a specific type, then return the default_value I specified.

So I’m thinking about changing the following method: https://github.com/nlohmann/json/blob/develop/src/json.hpp#L3653

to something like this:

    template <class ValueType, typename
              std::enable_if<
                  std::is_convertible<basic_json_t, ValueType>::value
                  , int>::type = 0>
    ValueType value(const typename object_t::key_type& key, ValueType default_value) const
    {
        // at only works for objects
        if (is_object())
        {
            // if key is found, return value and given default value otherwise
            const auto it = find(key);
            if (it != end())
            {
                try
                {
                    return *it;
                }
                catch (...)
                {
                    // if some kind of exception occurred, return default value
                    return default_value;
                }
            }
            else
            {
                return default_value;
            }
        }
        else
        {
            throw std::domain_error("cannot use value() with " + type_name());
        }
    }

Which I think will make the value method a lot easier to use. What do you think? @nlohmann

About this issue

Original URL
State: closed
Created 8 years ago
Reactions: 3
Comments: 48 (25 by maintainers)

Commits related to this issue

implemented a proposal for #278 — committed to nlohmann/json by nlohmann 8 years ago

Most upvoted comments

This bugs me as well, I would like to see much more permissives functions to avoid code like this:

auto it = object.find("property");
if (it != object.end() && it->is_string())
    myvalue = *it;

I would like to see much more permissives implicit conversions functions that also allow casting to default values if not.

json j; // null
int i = j; // i = 0

This will help us deserialize untrusted input much easier! Like this:

auto port = object["network"].value("port", 6667);

What do you think?

markand on Jul 20, 2016

It is very nice.

@jackb-p a behaviour change in a public function is considered as breaking change and thus should require a major version upgrade (if following semantic versioning).

For me there is no problem if it is a new function, it’s much more explicit.

markand on Aug 4, 2016

I implemented the proposal of @markand with a slight change: instead of having the user provide a type, the type is derived from the default value. There are two cases:

A value is found for the given key which matches the type of the default value; return the value at the key.
Return the default value otherwise.

An exception is only thrown if the function is called on a JSON value which is not an object.

There are still some issues with respect to number types: currently, -100 and 100 would be treated as different types. This may yield situations in which the return value is returned even if there is a stored value which just happens to be of the wrong number type…

The function (ignore the name for now) looks like this:

template <class ValueType, typename
          std::enable_if<
              std::is_convertible<basic_json_t, ValueType>::value
              , int>::type = 0>
ValueType value_with_type(const typename object_t::key_type& key,
                          ValueType default_value) const
{
    // derive JSON value type from given default value
    const value_t return_value_type = basic_json(default_value).type();

    // at only works for objects
    if (is_object())
    {
        // if key is found and stored value has the same type as the
        // default value, return value and given default value otherwise
        const auto it = find(key);
        if (it != end() and it->type() == return_value_type)
        {
            return *it;
        }
        else
        {
            return default_value;
        }
    }
    else
    {
        throw std::domain_error("cannot use value() with " + type_name());
    }
}

Here are some test cases:

json j =
{
    {"port", 1234}, {"proxy", true}
};

int port1 = j.value_with_type("port", 8080);
int port2 = j.value_with_type("the_port", 8080);
int port3 = j.value_with_type("proxy", 8080);

bool proxy1 = j.value_with_type("proxy", false);
bool proxy2 = j.value_with_type("the_proxy", false);
bool proxy3 = j.value_with_type("port", false);

CHECK(port1 == 1234);
CHECK(port2 == 8080);
CHECK(port3 == 8080);

CHECK(proxy1 == true);
CHECK(proxy2 == false);
CHECK(proxy3 == false);

What do you think?

Does this solve the issue for you? @cezheng, @jackb-p, @gregmarr, @prsyahmi, @markand ?
Should this be a separate function? If so, what should be the name?
Should this replace the current value function?

nlohmann on Jul 31, 2016

We could have a nothrow version For example in java json.getString(key) <- throw version json.optString(key, fallback value) <- nothrow version

prsyahmi on Jul 18, 2016

what about having a nothrow version of value so that users can choose which to use?

cezheng on Jul 13, 2016

What about only if the value is null? To me it makes sense for .value to return the default value if the value is null. For my usage, there is an API that returns JSON, but a certain string is set to null instead of not being present if it is not relevant, and I want to use the default value for this.

ghost on Jul 11, 2016