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

Check value for existence by json_pointer #1194

Closed
3galki opened this issue Aug 14, 2018 · 23 comments
Closed

Check value for existence by json_pointer #1194

3galki opened this issue Aug 14, 2018 · 23 comments
Labels
state: needs more info the author of the issue needs to provide more details state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated

Comments

@3galki
Copy link

3galki commented Aug 14, 2018

I have found no proper way to check element existence.
I can use json.at("..."_json_pointer) but it will throw an exception if it is missed.
I can use json.value() method. But I have to do it twice to check all cases (or use default that will not used in my json).

@nlohmann
Copy link
Owner

Internally, the JSON Pointer implementation also relies on exceptions. That is, there would be no real difference between implementing a find(const json_pointer&) interface as it would be literally the same as a calling at and coping with possible exceptions.

@3galki
Copy link
Author

3galki commented Aug 15, 2018

yep, now I have looked in code. So, looks like I have to avoid such logics...

@nlohmann
Copy link
Owner

Related: #1017

@ArnCarveris
Copy link

@nlohmann I need this json.find("..."_json_pointer)

@3galki
Copy link
Author

3galki commented Aug 21, 2018

json.find("..."_json_pointer)

Just one question, what value should be for end() in this case?

@ArnCarveris
Copy link

ArnCarveris commented Aug 22, 2018

@nlohmann Hmm good question, in mind answer is json.end(), but this is way not obvious.

Problem is what json.value("..."_json_pointer, /*default*/) throws exception when object in path not found.

I was forced to write this on my side:

std::istringstream s(key);
std::string k;

const Data* ptr = nullptr;

while (std::getline(s, k, '/'))
{
	if (k.empty())
	{
		ptr = &m_cData;
	}
	else if (ptr == nullptr)
	{
		return false;
	}
	else
	{
		auto it = ptr->find(k);
		
		if (it == ptr->end())
		{
			return false;
		}
		else
		{
			ptr = &it.value();
		}
	}
}

@nlohmann
Copy link
Owner

The issue is that iterators of different containers cannot be compared as this would be undefined behavior.

Therefore, writing code like

if (j.find("/foo/bar"_json_pointer) != j.end())
  ...

is not possible.

@ArnCarveris
Copy link

ArnCarveris commented Aug 24, 2018

@nlohmann Ok, maybe this: json.search<std::string>("/foo/bar"_json_pointer, [](const auto& value) {});?

@nlohmann
Copy link
Owner

Passing a lambda feels strange. I'd rather pass a std::pair<bool, json::iterator> with the bool indicating whether the search was successful (asserting the iterator can be dereferenced).

@ArnCarveris
Copy link

@nlohmann You said that with iterator no possible. Maybe this:

std::string value;
if (json.try_get(/foo/bar"_json_pointer, value))
{
 //do something when found
}

@nlohmann
Copy link
Owner

What would value mean in this situation?

@3galki
Copy link
Author

3galki commented Aug 27, 2018

You may try to get value() work without exceptions. It will help in most cases. Additional, you can add some value, that can't be found in json, and use it as default.
Or you can look at std::optional

@stale
Copy link

stale bot commented Sep 26, 2018

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 Sep 26, 2018
@stale stale bot closed this as completed Oct 3, 2018
@3galki
Copy link
Author

3galki commented Oct 4, 2018

will it resolved or just closed? ;(

@nlohmann nlohmann reopened this Oct 4, 2018
@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 Oct 4, 2018
@nlohmann
Copy link
Owner

nlohmann commented Oct 4, 2018

There is no solution for the issue yet. That's why it was closed eventually.

@nlohmann
Copy link
Owner

std::optional cannot be used as we are targeting C++11. I still have not understood how the function should look like.

@nlohmann nlohmann added the state: needs more info the author of the issue needs to provide more details label Oct 25, 2018
@3galki
Copy link
Author

3galki commented Oct 26, 2018

  1. You can do something like std::string::npos for your find (in this case all containers should have same end())
  2. You can make it by using std::optional but only for c++17 :)
  3. You can make your own "optional" class. And implement it by using std::optional; if c++17 or higher was used (stl already have implemented std::optional template)

@nlohmann
Copy link
Owner

  1. What should the function return in case of success?
  2. That is not an option.
  3. That is not really an option.

@3galki
Copy link
Author

3galki commented Oct 29, 2018

  1. the same as a typical find method returns (iterator). Yes. I haven't see absolutely good solution to.

Why you do not want to do your own std::optional? It would be modern solution (but should be not called find I think) :)

@ArnCarveris
Copy link

@nlohmann

Maybe this?
float* value = json.get_if<float>("..."_json_pointer);

@jaredgrubb
Copy link
Contributor

I don't like the idea that it would return a pair<bool, json::iterator> because at that point i would expect the iterator to have some kind of meaning. For example, if I took that iterator and did iterator-ish things to it, what does it mean? Sure, I can ask "it == end" to mean "not found", but what is it the end of? What does it mean if I increment that iterator? Which iterator concept does it model?

If you really want iterators, then I think you need to have a different kind of iterator, because you're modeling the structure very different. I think this is a bit like directory_iterator versus recursive_directory_iterator, where the first can only iterate across one level (like json::iterator), and the second can iterate across a full tree. I think drawing inspiration from might be helpful.

On the other hand, doing the get_if like @ArnCarveris suggests seems simple enough, but there's something about it that feels like it's too loose. For example, I'm effectively getting an interior pointer into the object, but knowing the rules on how long that pointer is valid would be very hard to pin down. But you could just say "the pointer is valid until any part of it or its surrounding containers (and so forth) are modified", and maybe that fixes that? I don't know. I think we'd have to think this through.

Does anyone have any prior art on other ways that boost or STL or some other solid library has offered a "reach inside some arbitrary heterogenous tree structure" has approached this? I can't imagine we're the first ones :)

@nlohmann
Copy link
Owner

I would not like to add our own implementation of std::optional to the library. As @jaredgrubb also pointed out: JSON pointers are not iterators - especially since they lack a means to describe the "past the end" position to indicate "not found". Unless we find a way to express the result of such a function, we cannot really proceed here.

@stale
Copy link

stale bot commented Dec 11, 2018

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 Dec 11, 2018
@stale stale bot closed this as completed Dec 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: needs more info the author of the issue needs to provide more details state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated
Projects
None yet
Development

No branches or pull requests

4 participants