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

Null safety/coalescing function? #1309

Closed
midn1 opened this issue Oct 20, 2018 · 9 comments
Closed

Null safety/coalescing function? #1309

midn1 opened this issue Oct 20, 2018 · 9 comments
Labels
solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@midn1
Copy link

midn1 commented Oct 20, 2018

Describe the feature in as much detail as possible.
A null coalescing function, similar to that of the null coalescing operator found in most modern languages (?? in C#, ?: in Kotlin, || in JS almost). Except in here it would be a template function named or, or something similar.
I believe this would make code a bit cleaner, as there would be less ifs and temporary variables scattered around the place.

Include sample usage where appropriate.

#include"json.hpp"
#include<iostream>
int main() {
    nlohmann::json j = R"({"x": 5, "y": null})"_json;
    std::cout << j["x"].or(29) << ' ' << j["y"].or(10) << std::flush;
}

Expected output would be:

 5 10
@nlohmann
Copy link
Owner

For objects, we have a value function like Python.

@nlohmann
Copy link
Owner

Here is the documentation of the value function.

I think the proposed code is a bit dangerous: What should happen in case of j["z"].or(10)?

  • If j is non-const, then null is added to the object at key z, and 10 would be returned by the or function.
  • If j is const, then access at a non-existing key is undefined behavior.

That's why I am not convinced that a member function or is a good idea, because we cannot make a check whether the given key exists in the first place.

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Oct 21, 2018
@timprepscius
Copy link

I'm not writing this to be annoying. I fully agree with you on following the std::map's behavior of j["not-a-key"], from the json_pointer thread...

But, if the j["not-a-key"] did not create a key/nullvalue but generated a deferred key-reference class, (which would then set the key on operator= or throw an exception on operator T), this problem of .or would also go away, as the deferred key could, as well, have the .or function.

@nlohmann
Copy link
Owner

We implement the operator[] for maps just like for arrays: no range check is performed; that is, no overhead is produced in case the client code knows the key is present.

@RokerHRO
Copy link

RokerHRO commented Oct 22, 2018

I don't like your "magic or() function approach", regardless that or is a keyword in C++ and cannot be used as a function name at all.

If you insist of having a function that "gets a value or return a default value", I'd name it this way:

j.get_or_default_value("key", compiletime_default_value)

Why? Well, always remember: You don't write the code for yourself! If your project is getting successfull, there will be another guy who might be your successor or a maintainer who is hunting for a bug. And – obviously, because you are the best C++ hacker on earth – their C++ knowledge is far less than yours, but they also have to understand your code, even at 3 a.m.!

So I'd always prefer functions with clear names instead of magic syntax or awkward overloaded operators. :-)

@nlohmann
Copy link
Owner

@RokerHRO This function exists and is called valued, using the name from Python's dict.

@RokerHRO
Copy link

@nlohmann : Excellent! Okay, I would have chosen another name, because value() is a quite generic name and I don't know Python, so I have no associations with Python's dict. But …meh…, I can live with that. :-)

@nlohmann
Copy link
Owner

@lvivtotoro Can I close this issue in favor of the value function?

@nlohmann nlohmann added solution: proposed fix a fix for the issue has been proposed and waits for confirmation and removed state: please discuss please discuss the issue or vote for your favorite option labels Oct 25, 2018
@midn1
Copy link
Author

midn1 commented Oct 25, 2018

Yup.

@midn1 midn1 closed this as completed Oct 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

4 participants