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

Replace assert with throw for const operator[] #1614

Closed
terminatorul opened this issue May 30, 2019 · 5 comments
Closed

Replace assert with throw for const operator[] #1614

terminatorul opened this issue May 30, 2019 · 5 comments

Comments

@terminatorul
Copy link

Is is possible please to change const operator[] to throw if field name does not exist, instead of declaring this case as "undefinded" and then using assert ?

@nlohmann
Copy link
Owner

This would be a breaking change, so no.

@sogartar
Copy link

@nlohmann , I don't understand how this is a breaking change if the behavior is undefined. It can be anything. You should not get to the undefined behavior point anyway. If it throws it is just restricting the "beyond this point everything is possible" to only throwing.
The thing that would change is the burden of another bounds check.

@nlohmann
Copy link
Owner

But operator[] is not supposed to do bound checking.

@terminatorul
Copy link
Author

It saves users the tedious task of tracking when the json object is const, and if so then checking before access if member exists. And if not found, maybe throw something :) or otherwise return.

For objects, the run-time overhead of my explicit check is duplicated work, because the operator will do the search in the properties map any way.

Maybe more important is keeping the behavior consistent with user expectations: if the non-const operator is always safe, how can the const one crash ? For most users out there, the distinction between const and non-const definitely should not cause such behavior.

Imagine my surprize when the QA team reported my latest fix was taking the webserver down. When all I did was to add const to a parameter that I do not change, and with a catch(...) clause carefully in place.

@nlohmann
Copy link
Owner

You are aware of at?

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