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

About value(key, default_value) and operator[](key) #1358

Closed
oyoungs opened this issue Nov 15, 2018 · 4 comments
Closed

About value(key, default_value) and operator[](key) #1358

oyoungs opened this issue Nov 15, 2018 · 4 comments
Labels
solution: invalid the issue is not related to the library

Comments

@oyoungs
Copy link

oyoungs commented Nov 15, 2018

When I use a json object(it may be object or null), I want to use the interface "value(key, default_value)" to get the property of the key, but if the json is null, the call will throw exception like "null cannot use value()", I dont't think it is better...
Why not change it to be used like:

nlohmann::json json /* = ... , from others*/;

// json may be null, or an object

std::string name = json.value("name", ""); // I want  no exception thrown, the name must be some string value or empty string

// and when I use operator[](key) 
void function(const nlohmann::json& args) {
    auto infomation = args["info"];
    if(infomation.is_null()) {
        // some work if it is null, maybe I will use default information 
    }

    if(information.is_object()) {
       // some work if it is an object, maybe I will use default value for some keys in the information object
    }
}

But after I upgraded it to the newest version several days ago, more exception is thrown, and more assertions make the old programs crash... T_T

Why make the change?

@nlohmann
Copy link
Owner

  • Can you provide more information on the exceptions you encountered after the update?
  • Can you provide the code that ran without issues before, but crashes now?
  • Which version and compiler are you using?

@nlohmann nlohmann added the state: needs more info the author of the issue needs to provide more details label Nov 15, 2018
@oyoungs
Copy link
Author

oyoungs commented Nov 15, 2018

I update the header file by pulling the source code and building install.

Before I update, I used the operator[](key) to get a nullable value from one const json reference, it worked very well (already so many code used) , just like this:

bool check_valid_args(const nlohmann::json& args)
{
    const auto& info = args["info"];
    if(!info.is_null() && !info.is_object()) return false;
    
    const auto& name = args["name"];
    if(!name.is_null() && !name.is_string()) return false;
    return true;
}

But after pulling the new source code and building install, all I used like that does't work any more, and make the programs crash for the assertion "object.find(key) != object.end()"

Why change the usage?

Is it better to return a json(null) when I call operator[](key) from an object without the key/value?

If the change is necessary, the only way for me to avoid crash is to rollback to old version, otherwise too many crashes to fixed T_T

@nlohmann
Copy link
Owner

In your code, you call operator[] on a const json value. This operator performs no bound check and its behavior is undefined in case the given key was not found (see documentation). This behavior is unchanged since version 1.0.0. In debug mode, the undefined behavior is guarded by the assertion you encounter. In release mode, the function does whatever dereferencing an invalid iterator does.

In contrast, the non-const version of operator[] behaves similar to std::map and returns a reference to a default constructed element in case a given key was not found. In case of json, this would be a null value. Only in that case, if (!info.is_null()) makes sense.

So:

  • The behavior of the library did not change.
  • The function always relied on undefined behavior.
  • Maybe you only compiled the updated version in debug mode so the assertion surfaced only now.

You should use is_object() to make sure args is an object and find to make sure info or name are keys in the object.

@nlohmann nlohmann added solution: invalid the issue is not related to the library and removed state: needs more info the author of the issue needs to provide more details labels Nov 15, 2018
@oyoungs
Copy link
Author

oyoungs commented Nov 16, 2018

Thanks! It helps me a lot ^_^

As I wrote before, I wanted to use the search for a key as little as possible, but use the found result more times. Once search more use .

It takes twice search time at least that find first and then use operatot[](key) , if the total count of keys is too large, it will take much more times. So I used it like that

Maybe I should change my usage.

I write a util function to make the old code compatible and use the correct usage you described since now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution: invalid the issue is not related to the library
Projects
None yet
Development

No branches or pull requests

2 participants