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

Port has_shape from dropbox/json11 #528

Closed
berkus opened this issue Mar 21, 2017 · 8 comments
Closed

Port has_shape from dropbox/json11 #528

berkus opened this issue Mar 21, 2017 · 8 comments

Comments

@berkus
Copy link
Contributor

berkus commented Mar 21, 2017

It has a very nice utility function, which I ported to nlohmann/json for myself, but would be nice to have it as part of the class:

#pragma once

#include <json.hpp>

/* has_shape(json, types)
 *
 * Return true if this is a JSON object and, for each item in types, has a field of
 * the given type. If not, return false and set err to a descriptive message.
 *                                        ^^^ feel free to adjust the return to your error model, but this one is handy
 */
using shape_t = std::initializer_list<std::pair<std::string, nlohmann::json::value_t>>;

bool has_shape(nlohmann::json const& js, shape_t const& shape, std::string& err)
{
    if (!js.is_object()) {
        err = "expected JSON object, got " + js.dump();
        return false;
    }

    for (auto & item : shape) {
        if (js[item.first].type() != item.second) {
            err = "bad type for " + item.first + " in " + js.dump();
            return false;
        }
    }

    return true;
}
@berkus
Copy link
Contributor Author

berkus commented Mar 21, 2017

Usage:

auto check = shape_t {
  {"name", json::value_t::string},
  {"ttl",  json::value_t::number_unsigned}
};

if (has_shape(js, check, err)) {
  cout << "Conforming object" << endl;
} else {
  cout << "Non-conforming object: " << err << endl;
}

@TurpentineDistillery
Copy link

  • The function should not modify the passed object (both args should be const-reference).
  • The function should not invoke undefined behavior (trying to access by missing key).
  • I'd expect such function to be recursive and applicable to all json-types, not just json-object.
  • In case of json-array, it is unclear what the behavior should be, since the semantics of a json-array may be either a list (any size, elements of a uniform shape) or a tuple (fixed size, elements of any shape).
  • In case of signed-integer data, it is unclear whether/when it should type-match, e.g. if shape is unsigned, query-type shall be also unsigned; if shape is signed, the query may be either signed or unsigned.
  • If the expected type is floating-point, it is unclear whether it should type-match with query type of signed/unsigned/null.
  • output-message parameter should be optional.

@nlohmann
Copy link
Owner

I'm not sure whether such a function needs to be added. This sort of validation goes into the direction of JSON Schema or so, and is rather specific. Before we discuss specifics of a PR, let's talk about why this exact function is useful in which scenarios.

@berkus
Copy link
Contributor Author

berkus commented Mar 22, 2017

Well, it's a very convenient check that the object you received conforms to several assumptions at once.

Instead of checking field-by-field in a very clumsy manner

if (json.find(FIELD) != json.end and json.find(FIELD).type() == json::string) {
  json.find(FIELD).get<string>() do something with it
}

if (json.find(ANOTHER) != json.end and json.find(ANOTHER).type() == json::number_unsigned) {
  json.find(ANOTHER).get<int>() do something with it
}

if (json.find(THIRD) != json.end and json.find(THIRD).type() == json::number_unsigned) {
  json.find(THIRD).get<int>() do something with it
}
if (json.find(FOURTH) != json.end and json.find(FOURTH).type() == json::number_unsigned) {
  json.find(FOURTH).get<int>() do something with it
}

// it can be more than four fields here, e.g. a typical github api response would contain more than 10

you just specify expectation of what the object should look like and check in one go

if (!json.has_shape({
  {FIRST, string},
  {SECOND, number_unsigned},
  {THIRD, string}
  {FOURTH, array}
  {FIFTH, object}
})) 
   throw runtime_error("eh, not what i expected");

json[FIRST].get<string>() etc you can use further without any checks - it's there

I find this pattern repeatedly convenient when using json, even if I don't require full-fledged schema validation. It's just less to type and less syntax errors to make. (if json.find(x) != json.end pattern is good for library writers but very inconvenient to type for application developers, it should have been something as simple as json.has_field(name, type))

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Mar 22, 2017
@nlohmann
Copy link
Owner

Any thoughts on this?

@meox
Copy link

meox commented Mar 30, 2017

I think that the proposal is great but probably is better if we can see in "embedded" style:

json::match_shape({
   {FIRST, [](auto&& x){ ...  } },
   {SECOND, [](auto&& x){ ...  } },
...
});

I use an approach similar to that on a simple messaging system based Actor Model (powered by ZMQ & nlohmann::json). I think that this could transform a lot of code if few lines.

@nlohmann
Copy link
Owner

It seems this check can be realized entirely with public methods, right?

@nlohmann
Copy link
Owner

nlohmann commented Apr 5, 2017

I don't see this as an extension to the library that is useful for many. It is a very special use case, makes only sense for JSON objects, and after all, it can be realized entirely with public methods.

Therefore, I tend to close this issue unless I missed something.

@nlohmann nlohmann closed this as completed Apr 9, 2017
@nlohmann nlohmann removed the state: please discuss please discuss the issue or vote for your favorite option label Apr 9, 2017
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

4 participants