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

feat: adds front and back methods to Value type #1458

Merged

Conversation

jwidauer
Copy link
Contributor

This PR adds the front and back methods to the Json::Value class.
These methods can be used to access the first and last elements of a Json::Value container.
For a Json::Value v they are equal to *v.begin() and *(--v.end()).

@BillyDonahue
Copy link
Contributor

BillyDonahue commented Jan 10, 2023

a Json::Value container.

Value is not a container. They're only valid if the Value is holding an array.
If the Value is holding an object, or anything scalar, this will fail, right?

@jwidauer
Copy link
Contributor Author

It should actually still be valid if Value is holding an object, since the iterator returned by Value::begin() is also valid for objects.
But, yes, for scalars it is undefined behavior as to what will happen, since you're trying to dereference a default constructed ValueIterator (or ConstValueIterator).
I didn't add an assertion, since it seemed like this is the preferred style, due to the fact that Value::begin() also just returns a default constructed iterator, which, when dereferenced, will fail.

@jwidauer
Copy link
Contributor Author

@BillyDonahue Does the previous comment answer your question?

include/json/value.h Outdated Show resolved Hide resolved
include/json/value.h Outdated Show resolved Hide resolved
src/lib_json/json_value.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@BillyDonahue BillyDonahue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more small request. otherwise good to go, I think.

src/lib_json/json_value.cpp Outdated Show resolved Hide resolved
include/json/value.h Show resolved Hide resolved
@jwidauer jwidauer force-pushed the add-front-and-back-methods branch from d7e4986 to e9a5ddb Compare January 31, 2023 10:27
@BillyDonahue
Copy link
Contributor

Thank you! LGTM

@jwidauer
Copy link
Contributor Author

jwidauer commented Jun 7, 2023

@BillyDonahue What's the status on this? Is it still something you'd be interested in getting in?

@BillyDonahue
Copy link
Contributor

I guess it's safe to say Travis CI isn't going to finish :)

@BillyDonahue BillyDonahue merged commit 3d9bf8e into open-source-parsers:master Jun 7, 2023
@jwidauer jwidauer deleted the add-front-and-back-methods branch June 8, 2023 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants