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

size parameter for parse() #419

Closed
user1095108 opened this issue Jan 6, 2017 · 20 comments
Closed

size parameter for parse() #419

user1095108 opened this issue Jan 6, 2017 · 20 comments

Comments

@user1095108
Copy link

to save one call to strlen(), I think such a parse() function should be provided. A similar parse for char array, with size deduced is already provided.

@gregmarr
Copy link
Contributor

gregmarr commented Jan 6, 2017

When would you use this when you couldn't use the two iterator version?

@user1095108
Copy link
Author

There are many convenience parse()s already, why not this one as well?

@gregmarr
Copy link
Contributor

gregmarr commented Jan 6, 2017

If you have char *s, and its size, you can already do parse(s, s+size). Are you looking for something different?

@user1095108
Copy link
Author

just a convenience function, what you describe is exactly what I want.

@gregmarr
Copy link
Contributor

gregmarr commented Jan 6, 2017

Well, it's up to @nlohmann but I don't see that the convenience benefit for parse(s, size) compared to parse(s, s+size) outweighs the cost, as additional overloads increase complexity and the possibility of ambiguity.

@nlohmann
Copy link
Owner

nlohmann commented Jan 6, 2017

The parse(s, s+size) constructor is much more versatile, because it allows to take any iterator range. An additional iterator parse(s, size) would just copy a subset of the features of the former.

Personally, I think the former feels more C++ like, whereas the latter feels more like C. But ymmv.

@user1095108
Copy link
Author

good C is also C++

@nlohmann nlohmann closed this as completed Jan 6, 2017
@user1095108
Copy link
Author

There is a precedent for my suggestion:

http://en.cppreference.com/w/cpp/string/basic_string/basic_string

std::basic_string<> has such a constructor, that's about as C++ as anything.

@nlohmann
Copy link
Owner

nlohmann commented Jan 7, 2017

That's one of the ugliest interfaces of the STL ;)

@user1095108
Copy link
Author

Still, it invalidates your argument of my suggestion not being C++, STL is as C++-ish as it can be, but I agree parts of it are outdated and ugly. I'm certain, if your library were considered for standardization, the committee would add this parse() overload.

@nlohmann
Copy link
Owner

nlohmann commented Jan 7, 2017

Well, originally, we wanted to avoid a call to strlen. I guess this is achieved.

@user1095108
Copy link
Author

user1095108 commented Jan 7, 2017

Yes, it is achieved, but std::basic_string also accepts iterators in addition to

basic_string( const CharT* s,
size_type count,
const Allocator& alloc = Allocator() );

A user of the STL has all the reasons to expect such an overload, despite there existing 2 (or more) ways of achieving the same thing.

@TurpentineDistillery
Copy link

STL is not a pinnacle of modern c++ design - it has some ugly warts here and there, which is mostly due to legacy baggage (compatibility with C, pre-c++11). I think new/modern APIs that are not burdened with backward-compatibility promises should avoid/discourage legacy-style usage.

@user1095108
Copy link
Author

That's a subject to debate. I'd say many things, that you see even in boost aren't the pinnacle of modern c++. But, really, design is not an issue here. Your library largely follows the design of STL containers, it is STL-inspired. The user would therefore justifiably expect such an overload to exist.

@nlohmann
Copy link
Owner

nlohmann commented Jan 7, 2017

We are talking about a parse function and not a constructor. We have a lot of overloaded constructors to make the construction of the JSON values as intuitive and frictionless as possible. But a parse function has no equivalent inside the STL, so no one can just call parse "as expected". The overloads we do have a properly documented, and I think a comparison with std::basic_string is not really helping us.

@user1095108
Copy link
Author

okay, we will see what happens, if your library is accepted into the standard. I'm almost certain, this overload would be added, should that happen. Also, from my perspective, a constructor is just a special function.

@nlohmann
Copy link
Owner

nlohmann commented Jan 7, 2017

Currently, there is no effort in submitting the library to the standards committee. But if such an overload is an issue then, I am sure we can fix it :)

@user1095108
Copy link
Author

I think you're simply ignoring a part of C++ lore, that may be outdated, but you can't deny it's there.

@nlohmann
Copy link
Owner

nlohmann commented Jan 7, 2017

Then we agree to disagree.

@user1095108
Copy link
Author

I have a rival json library I wrote, I'll just publish it, with the said overload, of course.

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

4 participants