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

Finding a value in an array #399

Closed
pboettch opened this issue Dec 19, 2016 · 31 comments
Closed

Finding a value in an array #399

pboettch opened this issue Dec 19, 2016 · 31 comments

Comments

@pboettch
Copy link
Contributor

I started using nlohmann::json today (thanks for it) and I wanted to check whether a certain value is contained in an array: I used find(). It compiled but it does not work.

#include <json.hpp>
using json = nlohmann::json;
int main(void)
{
	json j({"hello", "world"});
	std::cerr << j << "\n";
	std::cerr << (j.find("hello") != j.end()) << " expecting 1\n";
	for (const auto &i: j)
		if (i == "hello")
			std::cerr << "found it while iterating\n";
    return 0;
}

it gives

["hello","world"]
0 expecting 1
found it while iterating

Reading json.hpp I saw that find() is often done with a key. What is the expected behavior of finding on an array?

@pboettch
Copy link
Contributor Author

Just looking a bit more - I think I found it ;-)

from json.hpp:

 iterator find(typename object_t::key_type key)
 {
     auto result = end();

     if (is_object())
     {
         result.m_it.object_iterator = m_value.object->find(key);
     }

    return result;
}

@nlohmann
Copy link
Owner

Yes, the documentation of find is also clear about this:

Finds an element in a JSON object with key equivalent to key. If the element is not found** or the JSON value is not an objec**t, end() is returned.

To find elements in an array, you can use std::find.

@pboettch
Copy link
Contributor Author

What confused me was the silent returning of find() giving me end(). Naively I'd expect an error somehow. I'm still discovering your style but would an assert not be the better solution for newbies like me. Please enlighten, it'll help me understand better your design decisions.

@nlohmann
Copy link
Owner

The design decisions are difficult, because we basically try to "merge" the behavior of a std::vector and a std::map. As find as member function is only available to std::map and I do not want to accidentally degrade to linear complexity, there is no member find for arrays.

I understand that silently returning end() is confusing. Maybe changing this to an exception could be a change for the 3.0.0 release.

@nlohmann nlohmann reopened this Dec 19, 2016
@nlohmann nlohmann added state: help needed the issue needs help to proceed state: please discuss please discuss the issue or vote for your favorite option labels Dec 19, 2016
@nlohmann
Copy link
Owner

Should the semantics be changed to throw in this case?

@gregmarr
Copy link
Contributor

gregmarr commented Dec 19, 2016

"The basic_json object represents a json object" is a precondition of calling this function.
Is there a general philosophy in this library on what to do in the case of a precondition violation?

@nlohmann
Copy link
Owner

@gregmarr In similar issues where we mix the behavior of std::map and std::vector we have the range of

  • defining the missing behavior
  • throw an exception if called on the "wrong" value type

In this case, there is just a comment that when called on anything but an object, find will always behave as if nothing was found.

As searching in an array is rather expensive, I tend to throw an exception in this case.

@pboettch
Copy link
Contributor Author

I just ran into the same problem with

size_type size() const noexcept

for a string-element. I accidently took it for the length of the string.

@nlohmann
Copy link
Owner

@pboettch This is also clearly documented.

@pboettch
Copy link
Contributor Author

pboettch commented Dec 20, 2016

After some thinking I agree with you. find() and size() work as they should and my expectations were wrong. I even can explain what mislead me: The slogan: "What if json was part of modern C++?". I understood that json is trying to integrate as much as possible with std::-containers and algorithms, but no. It actually means what is says: how would we design json to be a container class in std::? Lite this.

As all cases are documented and well described, one just has to learn it. As for all the other containers.

You can close this "issue" from my point of view. I would have rather discussed this on a mailing list than in an issue. Thanks again for your work.

@TurpentineDistillery
Copy link

I may be going against the consensus here, but for someone who does not read documentation unless I can't get API to work to my expectations, returning end asserts "I searched for the value, and it's definitely not there", whereas throwing domain_error says misuse!

@whackashoe
Copy link
Contributor

I don't see a major issue with allowing find on arrays. If there is a note regarding the complexity impact that would be nice. I am picturing a structure with maybe 2-10 items in it, each referencing some id, and upon each we want to see if it contains x and y. Writing a search function by hand for each of these isn't convenient, but it's exactly what we want. Having it return end is confusing. Having it throw an exception isn't useful. There is already a heavy slant of usability over performance, but even the implementation of this shouldn't add much if any impact.

A little bit of this is my wish for more convenience methods in the standard so take it for what you will ✌️ Since json isn't an array nor a map having a perfect correspondence to one or the other depending on the subtype to me at least doesn't make a lot of sense.

@nlohmann
Copy link
Owner

So basically this is what I wrote in #399 (comment):

  • @TurpentineDistillery proposes to throw when find is used with an array ("throw an exception if called on the "wrong" value type")
  • @whackashoe proposes to actually search in an array and return an iterator and ("defining the missing behavior")

Any more opinions?

@pboettch
Copy link
Contributor Author

Keep it as it is? If you change find()-behavior on an array, why not change size()-behavior on a string?

Find is for finding a json-object identified by a key (AFAIU), in an array of strings there are no keys.

@nlohmann
Copy link
Owner

  • Right - I totally forgot about the signature of find.
  • I'm not sure about the semantics of size()...

@pboettch
Copy link
Contributor Author

My remark comparing a possible size()-behavior change to a find()-behavior-change was to show, IMHO, the current design is correct. It is strict and defined as one should expect.

I think my initial problem would have not appeared here if I had found an example of how to find a value in a json-array - for example in the README.

@whackashoe
Copy link
Contributor

Any of the associative containers (set, multiset, etc) are allowed conversion to a json array which do support find in their native type. Without allowing the ability to search once this type has been included then converted to an array it seems like a (albeit partial) lossy abstraction, instead this could be a additive abstraction which either gains or keeps equal the search method from the convertible types. Certainly either way leaving it with end being returned seems wrong.

@nlohmann
Copy link
Owner

So throwing seems to be the best way to go.

@nlohmann nlohmann removed the state: help needed the issue needs help to proceed label Dec 21, 2016
@nlohmann nlohmann added this to the Release 3.0.0 milestone Dec 21, 2016
@TurpentineDistillery
Copy link

TurpentineDistillery commented Dec 21, 2016

I don't insist on throwing. If find worked on vectors in linear time, that would be perfectly reasonable as well.

Edit: In fact, if you go with the principle of least astonishment, if a user called find on a json-array, I think they would be least astonished if it behaved like std::find called on a container.

as for size(), I think it should return the same value as if implemented as

    size_t size = 0;
    for(const auto& jj : my_json) {
        size++;
    }
    return size;

@nlohmann
Copy link
Owner

Good point. I use the semantics of the Container requirement: std::distance(begin(), end()).

@louwers
Copy link

louwers commented Dec 24, 2016

Letting find throw would definitely not be in congruence with the STL (std::map::find also returns end). But then again, having a find member on an array in the first place is not in congruence with the STL! So my opinion is that there shouldn't be such a member. My suggestion is to let people use std::find like you would have to when using std::arrays or std::vectors, unless there's some way a find member can be implemented that is less that O(N) (for objects this would work).

tl;dr: STL doesn't implement find for classes if it can't be done in less than O(N), json shouldn't either.

@TurpentineDistillery
Copy link

@louwers
size_t std::string::find(char c, size_t pos = 0) const noexcept;

@louwers
Copy link

louwers commented Dec 24, 2016

@TurpentineDistillery D'oh! Thanks for pointing that out. Lets try that again: the STL does't implement a find method for sequence containers. Because JSON values can be both a sequence container (an array) and an assosicative container (objects), I'm inclined to agree that a find method may be sensible, but it's debatable.

@nlohmann
Copy link
Owner

The find function is used quite frequently to check if a key is present in an object. Removing it would make that check quite cumbersome.

@TurpentineDistillery
Copy link

On a related note, if the behavior of find gets changed to throw or do the work, then behavior of count() should probably also change respectively.

@nlohmann
Copy link
Owner

The more I think about it, I tend to leave everything as is. The semantics of find is: "give me an iterator to the element with the given key, or end() if no such key exists". When called on a type other than an object, I think we satisfy this contract and return end() regardless of the key, as in: "You asked whether this integer 42 has an element with key "foo"? Well no it doesn't, here is an end() iterator". I think this is nothing we need to handle with an exception, because we can "answer" the question without any effort.

The same holds true for count(). "How many entries with key "foo" has this Boolean?" - "0.".

@nlohmann nlohmann removed this from the Release 3.0.0 milestone Dec 25, 2016
@nlohmann
Copy link
Owner

Another thing: I totally forgot the table in https://nlohmann.github.io/json which lists the behavior of all combinations of member functions known from the STL and the different JSON value types.

@nlohmann nlohmann added documentation and removed state: please discuss please discuss the issue or vote for your favorite option labels Jan 2, 2017
@nlohmann nlohmann added this to the Release 2.0.10 milestone Jan 2, 2017
@nlohmann nlohmann closed this as completed Jan 2, 2017
@zvrba
Copy link

zvrba commented Mar 21, 2018

I'm late to the discussion, but I've also just burned myself on this. I agree with people who think that .find() should throw if called on a non-object. There's already a precedent: .get<> throws when called on null json.

@nlohmann
Copy link
Owner

Changing the behavior now would be a breaking change, and I do not thing such a change would be for the better.

@nmz787
Copy link

nmz787 commented May 19, 2021

After searching online for about 10 minutes, I still can't figure out how to determine if a JSON array contains an element.

json j;
j["new"]["key"]["value"] = {"another", "list"};
std::cout<<j["new"]["key"]["value"].is_array()<<std::endl; 
std::cout<<j["new"]["key"]["value"].contains(std::string("another"))<<std::endl;

prints

1
0

what do I do here?

EDIT: here's my solution:

#define CONTAINS(list, value) std::find(list.begin(), list.end(), value) != list.end()
std::cout<<(CONTAINS(j["new"]["key"]["value"], "another"))<<std::endl;

@nlohmann
Copy link
Owner

contains() always returns false when called to JSON values that are not objects. Indeed, std::find is the right approach.

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

8 participants