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

Feature: to_string(const json& j); #916

Closed
DELTA37 opened this issue Jan 15, 2018 · 20 comments
Closed

Feature: to_string(const json& j); #916

DELTA37 opened this issue Jan 15, 2018 · 20 comments

Comments

@DELTA37
Copy link

DELTA37 commented Jan 15, 2018

It is very helpful to add method to_string(const json& j);

@theodelrieu
Copy link
Contributor

Agreed, would you like to open a PR? This one seems quite easy to implement :)

@nlohmann
Copy link
Owner

What would be the expected result? dump()?

@nlohmann nlohmann added state: please discuss please discuss the issue or vote for your favorite option and removed kind: enhancement/improvement labels Jan 15, 2018
@theodelrieu
Copy link
Contributor

That's what I would expect, do you agree @DELTA37?

@cronnosli
Copy link

I guess that .dump() is already enough, unless the intent is to replace .dump()!

@theodelrieu
Copy link
Contributor

The use-case I see would be something like the following:

// in some generic code
template <typename T>
void doStuff(T const& value) {
    using std::to_string;
    
    auto const str = to_string(value);
    // process the string
}

Other than that, I agree that dump() is pretty much enough.

@DELTA37
Copy link
Author

DELTA37 commented Jan 16, 2018

Yes, i guess to add "to_string()" method which will do the same as dump()
It is a standard method to extend std::to_string for user-defined class
And it is very helpful to do it with json

That is a discussion https://stackoverflow.com/questions/33399594/making-a-user-defined-class-stdto-stringable

// Sorry for my English

@DELTA37
Copy link
Author

DELTA37 commented Jan 16, 2018

#918
Attach pull request

@stale

This comment has been minimized.

@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 Feb 15, 2018
@stale stale bot closed this as completed Feb 22, 2018
@nlohmann nlohmann added good first issue and removed state: please discuss please discuss the issue or vote for your favorite option state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated labels Jan 13, 2019
@nlohmann nlohmann reopened this Jan 13, 2019
@stale

This comment has been minimized.

@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 Feb 12, 2019
@nlohmann nlohmann removed the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Feb 13, 2019
@stale

This comment has been minimized.

@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 Mar 15, 2019
@nlohmann nlohmann added pinned and removed state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated labels Mar 15, 2019
@Macr0Nerd
Copy link
Contributor

@nlohmann is this still available? It doesn't seem terribly important but it was marked as good first issue so :)

@nlohmann
Copy link
Owner

@Macr0Nerd Yes, it is! There were open issues in PR #918, so this issue was never fixed.

@Macr0Nerd
Copy link
Contributor

Alright! So @nlohmann all I need to is read a function that takes the JSON key, and pulls a deserializes the data into a key, correct? I’m still relatively green to C++ but I’ll give this my best!

@nlohmann
Copy link
Owner

No, a json value is given and to_string should return its serialization.

@Macr0Nerd
Copy link
Contributor

I think i see what you want (and based on the previous responses). It seems that you want to add an argument dependent lookup like this comment from the SO post from above. As it wasn't requested, I wouldn't overload the << operator but is this what you want @nlohmann?

I also apologize if I ask too many questions, I'm just of the mentality that it should be done right the first time (for the most part)

@Macr0Nerd
Copy link
Contributor

@nlohmann I believe I understand the purpose of this feature. It seems as if though the logic (and code) of dump would do the job here. I want to know if this is alright and what you want/expect

@Macr0Nerd
Copy link
Contributor

Well for lack of understanding and the fear of messing up a very useful product, I will be removing myself from this problem @nlohmann

@nlohmann
Copy link
Owner

Basically look at https://github.com/nlohmann/json/pull/918/files and follow the comments.

@Macr0Nerd
Copy link
Contributor

Macr0Nerd commented Apr 26, 2019

@nlohmann I put to_string in the nlohmann namespace as thats how the ADL for overwriting to_string works, but I saw that there needed to be some unit testing. Which file would I put the tests in? So far I've been putting it in unit-serialization after SECTION("dump").

@nlohmann
Copy link
Owner

Closed by merging #1585.

@nlohmann nlohmann added this to the Release 3.6.2 milestone May 24, 2019
@nlohmann nlohmann self-assigned this May 24, 2019
@nlohmann nlohmann modified the milestones: Release 3.6.2, Release 3.7.0 Jul 9, 2019
@nlohmann nlohmann added this to the Release 3.7.0 milestone Jul 28, 2019
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

5 participants