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

operator== documentation should show how to apply custom comparison function #1915

Closed
nmusolino opened this issue Jan 28, 2020 · 7 comments
Closed

Comments

@nmusolino
Copy link

What is the issue you have?

The documentation for operator == mentions that double NaN values compare unequal, and says, "To compare floating-point while respecting an epsilon, an alternative comparison function could be used..." The example function is for comparing two doubles.

In issue #514, about comparison between NaN values, @nlohmann wrote:

I think nothing must be changed here, only the documentation must be clear.

The documentation for operator == should illustrate how a custom double comparison function could be applied recursively to all values in a basic_json object. This would allow users to easily write their own function to evaluate two basic_json objects for equivalence.

I think such a function could be written using basic_json::flatten() and comparing items pairwise with the std::equal algorithm.

@nlohmann
Copy link
Owner

What I meant with "an alternative function" would be a user-defined function like

bool my_equal(const json& lhs, const json& rhs)
{
    switch (lhs.type())
    ...
}

which implements whatever logic you would like. There is no extension point to change the behavior of operator==.

@nmusolino
Copy link
Author

nmusolino commented Jan 28, 2020

I understand.

What I am trying to suggest is this... let's say that a user has a custom scalar comparison in mind, like the example in the documentation page, or like this:

inline bool is_same(double a, double b) noexcept
{
    return a == b || (std::isnan(a) && std::isnan(b)); 
}

Starting from that point, a user would like to evaluate whether two arbitrarily nested json objects are "equivalent," using the binary predicate. My suggestion is that the documentation include a working version of this function:

bool my_equal(const json& lhs, const json& rhs)
{
    // Uses `my_equal` to compare doubles.
    switch (lhs.type())
    ...
}

Maybe this could go on a "recipes" page if that is a better place for it.

By the way, I think that actually writing this function would be facilitated if json had a visit helper function as requested in #1458. Users could write a binary visit function on top of that.

@stale
Copy link

stale bot commented Feb 27, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Feb 27, 2020
@dota17
Copy link
Contributor

dota17 commented Mar 2, 2020

Comparing floating point with == directly is unsafe and dangerous, and it seems that compare their absolute value with epsilon is not perfect too, according to THE FLOATING-POINT GUIDE. @nmusolino @nlohmann Should we discuss how to deal with this situation?

@stale stale bot removed the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Mar 2, 2020
@nlohmann
Copy link
Owner

  • Regarding the visit helper function, please open an issue.
  • Changing the behavior of operator== would be a breaking change that I would like to avoid.
  • I agree that putting such examples into a dedicated places is a good idea, see Adding "examples" section for real-life usages #1953.

@Stannieman
Copy link

Stannieman commented Mar 20, 2020

I also think we should by default use the default operator== for all types.
Especially for floating point numbers there is no one size fits all way of comparing them.
Whether we should use normal ==, with epsilon or some more complex mechanism depends totally on the kind of data contained in the JSON.
Therefore, in my opinion, allowing the user to provide a custom way of comparing (whether that be a function or some other mechanism) is the only correct option here.

@nlohmann
Copy link
Owner

Closed by merging #1984.

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