add noexcept json::value("key", default) method variant?

[tyhenry]

the json::value("key", default) method is helpful, but allowing it to throw adds quite a bit of redundant try {} catch {} boilerplate - this can be especially onerous when e.g. loading config from sparse or error-prone json, where every value() call must be wrapped in a try{} block (in which case, why not just use the .at() method?)

A secondary noexcept value() function would be really helpful in this case - like json::value_safe("key", default).

string name = config_json.value_safe("name", "John");

rather than

string name;
try {
  name = config_json.value("name", "John");
}
catch (...) {    // throws if config_json.at("name") is null
  name = "John";
}

In my code I'm using the utility function:

// exception safe json value getter:
template<class ValueType, typename std::enable_if<
	std::is_convertible<nlohmann::json::basic_json_t, ValueType>::value, int>::type = 0>
inline ValueType json_value_safe(const nlohmann::json& j, const typename nlohmann::json::object_t::key_type& key, ValueType default_value) noexcept
{
	try {
		// if key is found, return value and given default value otherwise
		const auto it = j.find(key);
		if (it != j.end())
		{
			return *it;
		}
	}
	catch (...) {}
	return default_value;
}

[nlohmann]

Thanks for the proposal and sharing a concrete implementation! I understand you do not want an exception to be thrown. However, I am not sure whether it would be safe to mark your code noexcept, because return *it; would require the basic_json class to be is_nothrow_copy_assignable?

[jaredgrubb]

Technically, it would only require ValueType to be nothrow-copyable, no? You could embed that into the noexcept expression.

[tyhenry]

Thanks for the quick response! I had try to wrap my head around noexcept - but still a bit unsure.
Wondering though if changing ValueType to a const reference avoids the nothrow-copy issue?

this seems to work as expected for me (msvc 141):

template<class ValueType, typename std::enable_if<
    std::is_convertible<nlohmann::json::basic_json_t, ValueType>::value, int>::type = 0 >
inline const ValueType & json_value(const nlohmann::json& j, const typename nlohmann::json::object_t::key_type& key, const ValueType& default_value)
    noexcept
{
    try {
        // if key is found, return value and given default value otherwise
        nlohmann::json::const_iterator it = j.find( key );
        if( it != j.end() ) {
            return *it;
        }
    } catch(...) { }

    return default_value;
}

edit - I should add that I'm using this overload for const char *

inline nlohmann::json::string_t json_value( const nlohmann::json& j, const typename nlohmann::json::object_t::key_type& key, const char* default_value )
{
    return json_value( j, key, nlohmann::json::string_t( default_value ) );
}

[jaredgrubb]

No, because return default_value is returning a const& argument whose lifetime you cannot know.

For example, this compiles but will do nasty things:

{
     const std::string& result = json_value(something, "key", std::string{"Default Value"});
     // ^ may get bound to a dead temporary object
}

[tyhenry]

Thanks! That makes sense. Going back to the copy version, it seems to me @jaredgrubb you're right about ValueType only needing to be nothrow-copy-able. This seems to work correctly in my tests:

template<class ValueType, typename std::enable_if<std::is_convertible<nlohmann::json::basic_json_t, ValueType>::value, int>::type = 0 >
inline ValueType json_value( const nlohmann::json& j, const typename nlohmann::json::object_t::key_type& key, const ValueType& default_value )
noexcept( std::is_nothrow_copy_constructible<ValueType>::value )
{

    try {

        // if key is found, return value and given default value otherwise
        nlohmann::json::const_iterator it = j.find( key );

        if( it != j.end() ) {
            return *it;
        }

    } catch( ... ) { }

    return default_value;
}

Using the noexcept( std::is_nothrow_copy_constructible<ValueType>::value ) spec above, the following code runs fine. But using blanket noexcept that I had originally, std::terminate is called at return default_value;

class foo
{
public:
    foo() {}
    ~foo() {}
    foo( const foo & f ) { throw; }
};

inline void from_json( const nlohmann::json& j, foo& f )  { }
inline void to_json( nlohmann::json & j, const foo & f ) { }

int main()
{
    nlohmann::json j = {
        { "null", nullptr },
        { "foo", foo() }
    };

    // value null - return default
    int i = 0;
    try {
        i = json_value( j, "null", 1 );
    } catch( ... ) {
        std::cout << "threw at `i = json_value(j, \"null\", 1);`" << std::endl;
    }
    std::cout << "i = " << i << std::endl;

    // foo copy throws
    try {
        foo f = json_value( j, "foo", foo() );
    } catch( ... ) {
        std::cout << "threw at `f = json_value( j, \"foo\", foo() );`" << std::endl;
    }

    // key not found - return default (throwing copy)
    try {
        foo f = json_value( j, "no foo", foo() );
    } catch( ... ) {
        std::cout << "threw at `f = json_value( j, \"no foo\", foo() );`" << std::endl;
    }
}

prints

i = 1
threw at `f = json_value( j, "foo", foo() );`
threw at `f = json_value( j, "no foo", foo() );`

This behavior makes more sense to me than basic_json::value(), I'm wondering if it could be considered as a replacement or alternative like basic_json::value_safe()? Seems to expose a safe way of accessing/validating the json without the extra try blocks to catch bad values. If ValueType is not nothrow-copy-able, something like

f = json_value( j, "foo", foo() );

should be wrapped in a try block anyway.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.