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

Feature request: Implicit conversion to bool #951

Closed
wolframroesler opened this issue Feb 1, 2018 · 10 comments
Closed

Feature request: Implicit conversion to bool #951

wolframroesler opened this issue Feb 1, 2018 · 10 comments
Labels
state: please discuss please discuss the issue or vote for your favorite option

Comments

@wolframroesler
Copy link

What's the general opinion about allowing json objects in a boolean context? For example:

json j = nullptr;
if (!j) {
    std::cout << "The object is null\n";
}

if (j) should be the same as if (!j.is_null()), and if (!j) should be the same as if (j.is_null()).

In the present version, the code above throws an exception ("[json.exception.type_error.302] type must be boolean, but is object").

@nlohmann
Copy link
Owner

nlohmann commented Feb 1, 2018

But implementing operator bool() with return not j.is_null() has a problem with actual Boolean values:

json j = false;
if (j) {
    std::cout << "This output is surprising!\n";
}

@wolframroesler
Copy link
Author

You're right, bool is a special case, and your example shouldn't produce any output. How about something like this (to be understood as pseudo-code rather than something that could be dropped into json.hpp unchanged):

json::operator bool() const {
    return is_boolean() ? get<bool>() : ! is_null();
}

so bools work as expected, and for everything else, null is false and the rest is true.

They may be situations where you really don't know what type a json object is storing, in which case you'd probably like to use is_boolean, is_null, etc. explicitly. In my application, however, I usually know what I'm dealing with (for example, I know that something's either null or an object), so if (j) would work as expected, and the similarity to if (pointer) or if (stream) makes it feel natural.

Sounds intuitive, however that may also be because it's what I'm familiar with from another JSON library. Opinions?

@theodelrieu
Copy link
Contributor

This is related to an issue I need to open about the implicit conversion operator operator T.

While this operator exists, I think it is harmful to adopt such a change.

I will go in more details as soon as I find some time to open an issue.

@wolframroesler
Copy link
Author

I agree that implicit conversion is dangerous, that's why the C++ language avoids it whenever possible (think std::string::c_str). But conversion to bool is different, that's why smart pointers, streams, etc. can be used in a boolean context (and it's considered good style to do so -- I prefer if (!p) to if (p==nullptr), for example).

Of course, operator bool should be declared explicit to avoid some of the dangers, so that

json j = /* object */;
bool b = j;

won't work.

We should ask ourselves which solution suits best into the general C++ design philosophy (into which the JSON library integrates seamlessly, which is a major reason why I like it).

@theodelrieu
Copy link
Contributor

I agree, I personally would like to completely remove operator T, and to make json_object.get<> the only way to retrieve values.

But while there's this dreadful operator, I believe it would be too dangerous to add the behavior you're proposing

@wolframroesler
Copy link
Author

@theodelrieu do you have an example for unexpected things that would happen with the suggested bool conversion, provided that the existing operator T stays in place (which, I'm afraid, it will for compatibility reasons)?

@theodelrieu
Copy link
Contributor

Well first of all having return is_boolean() ? get<bool>() : ! is_null(); is very confusing.

You cannot know if what was returned was a correct boolean value, you have to call is_null to be sure, which defeats the original purpose.

Even without the operator T, I don't think it should be added in the end. Indeed, in the context of a JSON value, it's not clear what a conversion to bool should do. Even if the behavior is well documented, I fear it might confuse lots of people.

That's just my opinion right now, it might change in the future :)

@nlohmann
Copy link
Owner

nlohmann commented Feb 1, 2018

I am currently not really convinced that this operator would be helpful. However, it can be simulated with a function like

bool to_bool(const json& j) {
    return is_boolean() ? get<bool>() : ! is_null();
}

in user code. It's not an operator, but it could still simplify your code.

Any other ideas on this?

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Feb 1, 2018
@ColinH
Copy link
Contributor

ColinH commented Feb 2, 2018

I agree with @theodelrieu that the suggested return is_boolean() ? get<bool>() : !is_null(); is too confusing, even if there is a precedent for such behaviour. For example the Lua language adopts exactly the mentioned semantics, but then again, Lua has decidedly un-C++-like semantics nearly everywhere.

Regarding the comparison with smart pointers made by @wolframroesler, I'm not sure that the situation is similar enough. With smart pointers it is quite clear what it means to be "true". Here you have multiple possibilities, "what's the value of the contained bool?", "is the contained value not null?", the suggested combination "is it a bool that is true or not a null?".

The next person might want an empty array, and an empty object, perhaps even an empty string, to return false, too. Or why doesn't operator bool return false when it contains an integer that is zero, aka. the good old C semantics that everybody is familiar with and some might expect here?

When there is such ambiguity due to multiple valid choices I would argue that it is often better to not implement any one of them. This minimises the number of people being surprised by behaviour that doesn't match their expectation which, after all, would still be "a" valid one, even when not "the chosen one".

@wolframroesler
Copy link
Author

Point taken. Issue revoked. Thanks for your explanations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: please discuss please discuss the issue or vote for your favorite option
Projects
None yet
Development

No branches or pull requests

4 participants