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

NaN-like comparison behavior of discarded is inconvenient #1988

Closed
jbms opened this issue Mar 16, 2020 · 6 comments
Closed

NaN-like comparison behavior of discarded is inconvenient #1988

jbms opened this issue Mar 16, 2020 · 6 comments

Comments

@jbms
Copy link

jbms commented Mar 16, 2020

The special discarded value has NaN-like comparison behavior, i.e. discarded != any value, including discarded, but I think it would be better for it to have normal comparison behavior, where discarded == discarded, discarded != any other value.

I have found discarded to be convenient for representing a missing object member in code that parses an ::nlohmann::json value into a custom type, but the NaN comparison behavior makes that usage more complicated and more error prone.

It also potentially breaks std::map<::nlohmann::json, T>.

I think in practice the NaN comparison behavior is unlikely to be useful, and likely to be confusing.

@jbms jbms added the kind: bug label Mar 16, 2020
@Xeverous
Copy link

I find this to be very unintuitive:

auto json = nlohmann::json::parse(str, nullptr, false);
if (json == nlohmann::json::value_t::discarded) // ALWAYS false
if (json.is_discarded()) // actual proper test, different behavior than operator==

especially when https://nlohmann.github.io/json/classnlohmann_1_1basic__json_a265a473e939184aa42655c9ccdf34e58.html#a265a473e939184aa42655c9ccdf34e58 literally says "Returns
deserialized JSON value; in case of a parse error and allow_exceptions set to false, the return value will be value_t::discarded.". Any user would then expect that a such failed-parse JSON will compare equal to discarded value.

Also, the documentation on https://nlohmann.github.io/json/classnlohmann_1_1basic__json_aabe623bc8304c2ba92d96d91f390fab4.html#aabe623bc8304c2ba92d96d91f390fab4 - "This function returns true if and only if the JSON value was discarded during parsing with a callback function" is incorrect because json.is_discarded() is true for failed parse even if the callback function was not provided and default value (nullptr) was used.

@nlohmann
Copy link
Owner

I'm afraid the behavior of operator== is like this since 1.0.0, so changing it now may break a lot of existing implementations. The only "bug" that I can see is that the behavior for discarded values should be stated explicitly in the documentation of operator==. Discarded values are a means from the library to communicate something went wrong in situations where exceptions are not applicable (and the null value cannot be used, because null would have a different meaning). Using discarded values in client code was never intended by the library. This may also be stated in the documentation.

@Xeverous
Copy link

Using discarded values in client code was never intended by the library.

How can I then parse with allow_exceptions = false and check whether parse succeeded or not? So far .is_discarded() does the job.

@nlohmann
Copy link
Owner

Yes, is_discarded() is meant for this.

@jbms
Copy link
Author

jbms commented Mar 21, 2020

I suspect that few users rely on the behavior of operator==, so it might still be feasible to change it when other breaking changes are also made, e.g. to remove implicit conversion from json to other types.

Alternatively, it might be useful to provide an additional comparison function (same?, identical?) that behaves like operator== but also has discarded == discarded.

@nlohmann
Copy link
Owner

The documentation of operator== already shows an example for an alternative comparison function. See also #1915 #1984. I do not think that adding such a function to the library brings much benefit.

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