Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

object field accessors #103

Closed
ropuls opened this issue Jul 3, 2015 · 4 comments
Closed

object field accessors #103

ropuls opened this issue Jul 3, 2015 · 4 comments

Comments

@ropuls
Copy link

ropuls commented Jul 3, 2015

Hi Niels,

not sure if this is needed inside the class, however that pattern is widely in use here, and might be of use for other users as well.

template <typename T>
T get(json &value, const std::string &key, T defval = T()) {
    if (value.is_object()) {
        auto it = value.find(key);
        if (it != value.end()) {
            try {
                return it.value().get<T>();
            } catch (...) {

            }
        }
    }
    return defval;
}

void dispatch(const json &j) {
    std::string op = get<std::string>(j, "op");
    if (op == "replace") {
        ...
    } else if (op == "insert") {
        ...
    } else if (op == "select") {
        ...
    }
}

In a real implementation, names would need to be changed, and SFINAE type-checking shall be made if T is generally convertible from json's underlying types. It would also be good to avoid the internal exception, if possible, and have a non-throwing path (for me an exception is almost ever an error condition, whilst that new accessors gently asks "may I have it as T if that key is here"?

Another variant for c++14 and beyond users would be:

template <typename T>
std::optional<T> get(const json &j, const std::string &key) {
    ...
}

if (get(j, "op").value_or("")) { ... }

Which would make things even better/more precise.

Cheers,
Roman

PS: with gcc5 (stolen from their homepage) we can detect std::optional like this:

#ifdef __has_include
#  if __has_include(<optional>)
#    include <optional>
#    define have_optional 1
#  elif __has_include(<experimental/optional>)
#    include <experimental/optional>
#    define have_optional 1
#    define experimental_optional
#  else
#    define have_optional 0
#  endif
#endif
@nlohmann
Copy link
Owner

nlohmann commented Jul 4, 2015

Hi Roman,

thanks for checking back. I understand why such a function would be interesting.

I have one question: What is the advantage of a non-member function T get(json &value, const std::string &key, T defval = T()) over the member function T json::get(const std::string &key, T defval = T())?

As for implementation, I would try to avoid the try/catch block as follows: I would create five versions of the function (boolean, array, object, number, string) and use SFINAE to select the right one based on the possibility to convert from the respective JSON type (json::booleant_t, json::array_t, ...) to typename T. Then I would use find to check if the passed key is present and get_impl_ptr to get a pointer to the value (or nullptr if the JSON value has the wrong type).

What do you think?

I would like to avoid std::experimental::optional, because I would leave the C++11 world - it is already hard if not impossible to get all this running on MSVC. I keep the idea in mind, though, and once there are enough features with a simpler C++14 solution, one could think of moving to C++14 altogether.

Cheers,
Niels

@arnaudbrejeon
Copy link

I find myself using the "get with default value if missing" quite often as well. It would be a great addition to the member functions. For booleans I use something like this:

static bool getBoolValueWithDefault(const json& jsonC, const std::string& key, const bool& default_value) {
return jsonC.is_object() == false ? default_value : jsonC[key].is_boolean() ? jsonC[key].get() :
default_value;
}

@nlohmann
Copy link
Owner

I think this could be solved with #133.

@arnaudbrejeon
Copy link

Great! Thank you for the good work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants