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

Implement std::visit interface for json values. #71

Closed
Siapran opened this issue Jan 10, 2020 · 6 comments
Closed

Implement std::visit interface for json values. #71

Siapran opened this issue Jan 10, 2020 · 6 comments

Comments

@Siapran
Copy link

Siapran commented Jan 10, 2020

It would be nice to be able to use visitors on json values like you would with std::visit.
see: https://en.cppreference.com/w/cpp/utility/variant/visit

ColinH added a commit that referenced this issue Jan 11, 2020
@ColinH
Copy link
Member

ColinH commented Jan 11, 2020

It seems to be easy, please try with the latest commit - and let's hope that I didn't miss any important reason why it can't always be so easy :-)

@d-frey
Copy link
Member

d-frey commented Jan 11, 2020

Are we sure we want to do it this way? We could also expose the variant with an accessor (getter) and instead of v.visit( vis ); you'd write std::visit( vis, v.variant() );. Benefit: You can have visitors to visit multiple variants at once, the exposed variant can be queried for .index(), etc. and you can use other standard methods on it. I guess the real question we need to ask is: How much is m_variant an implementation detail or how much do we want to expose it?

(I even considered deriving from std::variant briefly, but that might have other implications.)

@Siapran
Copy link
Author

Siapran commented Jan 11, 2020

alternatively, specializing or providing the interface needed for std::visit (std::get and index I believe?)

@d-frey
Copy link
Member

d-frey commented Jan 11, 2020

Unfortunately that is not allowed, std::get is not a customization point from the standard's point of view. (I would have preferred it, but yeah, not allowed)

@ColinH
Copy link
Member

ColinH commented Jan 11, 2020

@d-frey Yes, how far do we want to expose the variant? I guess at this point there aren't too many reasons to keep it private? We advertise as being standard containers based, we use the standard string classes, so we might as well complete the picture. And then offering a getter could replace visit().

ColinH added a commit that referenced this issue Jan 11, 2020
@ColinH
Copy link
Member

ColinH commented Jan 11, 2020

Since it seems to fit with both the design of the library and the code we decided to implement a getter called basic_value::variant() instead of basic_value::visit(), which is more flexible.

@Siapran Siapran closed this as completed Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants