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

How to construct an iteratable usage in nlohmann json? #636

Closed
intijk opened this issue Jun 24, 2017 · 31 comments
Closed

How to construct an iteratable usage in nlohmann json? #636

intijk opened this issue Jun 24, 2017 · 31 comments
Labels
kind: enhancement/improvement kind: question state: please discuss please discuss the issue or vote for your favorite option

Comments

@intijk
Copy link

intijk commented Jun 24, 2017

Recently I am thinking to have some thing like this,

Foo a, b, c;
json j=json(Array());
j.push_back(&a);
j.push_back(&b);
j.push_back(&c);

for(auto e: j){
//e would get a pointer to Foo class.
}

Is this possible to implement? I tried to_json and from_json , but seems the features are limited.

@cursey
Copy link

cursey commented Jun 24, 2017

If you're storing pointers in the json object just cast them to something like uintptr_t and then cast them back when iterating over them. from_json and to_json are for constructing objects/json objects, not pointers to them.

@gregmarr
Copy link
Contributor

This seems like a really bad thing to do. Why would you not use std::vector<Foo *>? The pointers are only good for as long as the Foo objects remain live, so the j object has to have a lifetime that is less than or equal to the Foo objects, and you have to jump through all kinds of hoops to get the pointers into and out of JSON. What are you going to be doing with j where you want to have pointers in it?

@intijk
Copy link
Author

intijk commented Jul 5, 2017

This seems like a really bad thing to do. Why would you not use std::vector<Foo *>? The pointers are only good for as long as the Foo objects remain live, so the j object has to have a lifetime that is less than or equal to the Foo objects, and you have to jump through all kinds of hoops to get the pointers into and out of JSON. What are you going to be doing with j where you want to have pointers in it?

The reason is that I kind of want enjoy the easy management and pass of a flexible json object, with that object I could easily bind the tag and other related thing together, but with vector, I have to use different type and glue, which is not look good as json.

@nlohmann
Copy link
Owner

nlohmann commented Jul 5, 2017

There is no way to store pointers as JSON values, unless you want to covert them to 64 bit unsigned integers.

@intijk
Copy link
Author

intijk commented Jul 6, 2017

OK, I will try to have some pointer store mechanism and try.

I think nlohmann json is so close to enable c++ to deliver something like python. The only barrier now is the pointer store mechanism. If nlohmann json could enable some thing like

Foo a, b, c;

j.push_back(a);
j.push_back(b);
j.push_back(c);

for(auto f:j){
     f.fun();
}

There is no need to use anything like iterator .

@nlohmann
Copy link
Owner

nlohmann commented Jul 7, 2017

That example would not work, because the range for (you must wrap j with the iterator_wrapper) returns a reference of type basic_json, which does not know fun().

@intijk
Copy link
Author

intijk commented Jul 7, 2017

Yes, I know it not work.

I just want to know if it is possible to do this in nlohmann json, I would very glad to see this as a useful extension, this could potentially change the way how we write cpp code.

@nlohmann
Copy link
Owner

nlohmann commented Jul 7, 2017

I understand, but I fear that this is out of scope of this library. It would require a lot of extensions that are unrelated to JSON. And I am not sure whether the example you provided is actually possible in C++ without pointers and reinterpret_casts.

@intijk
Copy link
Author

intijk commented Jul 7, 2017

That is just my suggestion, I would try some thing weak version to enable the usage here

Foo a,b,c;

j.push_back(&a);
j.push_back(&b);
j.push_back(&c);

Foo* p = (Foo*)(j[1]);

The drawback is that user still need to know type of the pointer.

@nlohmann
Copy link
Owner

nlohmann commented Jul 7, 2017

Then again: storing pointers has nothing to do with JSON.

@intijk
Copy link
Author

intijk commented Jul 7, 2017

I know I need to convert it to something like uint64_t

@nlohmann
Copy link
Owner

nlohmann commented Jul 7, 2017

You may use something like this:

#include "json.hpp"

using json = nlohmann::json;

class Foo
{
  public:
    void bar() { std::cout << "bar()" << std::endl; }
};

void to_json(json& j, const Foo& f) {
    j = reinterpret_cast<uint64_t>(std::addressof(f));
}

void from_json(const json& j, Foo& f) {
    f = *reinterpret_cast<Foo*>(j.get<uint64_t>());
}


int main()
{
    Foo f;
    f.bar();
    
    json j = f;
    
    Foo f2 = j;
    f2.bar();
}

@nlohmann
Copy link
Owner

nlohmann commented Jul 8, 2017

I close this issue as there is nothing to be done for the library.

@intijk
Copy link
Author

intijk commented Jul 8, 2017

Thank you for the example code, it is very useful.

@intijk
Copy link
Author

intijk commented Jul 8, 2017

I am trying to modify a little bit of the code and use pointer type to avoid the copy constructor of Foo. I tried the code like this:

#include "json.hpp"

using json = nlohmann::json;

class Foo
{
  public:
    void bar() { std::cout << "bar()" << std::endl; }
};

void to_json(json& j, const Foo* f) {
    j = reinterpret_cast<uint64_t>(f);
}

void from_json(const json& j, Foo*& f) {
    f = reinterpret_cast<Foo*>(j.get<uint64_t>());
}


int main()
{
    Foo f;
    f.bar();
    
    json j=&f;
    
    Foo *f2;
    f2=j;
}

But I meet below error:

tt.cc: In function ‘int main()’:
tt.cc:28:4: error: cannot convert ‘json {aka nlohmann::basic_json<>}’ to ‘Foo*’ in assignment
  f2=j;

Is it possible for the Foo* as left value of assignment?

@nlohmann
Copy link
Owner

nlohmann commented Jul 8, 2017

I'm not sure. Maybe @theodelrieu has an idea. He wrote the whole custom-type conversion.

@theodelrieu
Copy link
Contributor

Hi, I think you can achieve this, by specializing adl_serializer:

(I did not compile any of this though)

namespace nlohmann {
// You can also fully specialize on Foo*
template <typename T>
struct adl_serializer<T*> {
static void to_json(json& j, const T* f) { /* ... */ }
static void from_json(const json& j, T*& f) { /* ... */ }
};
}

Let me know if that works!

@intijk
Copy link
Author

intijk commented Jul 8, 2017

Hi I tried the code you provide , still report the same error, I can assign to json, but not assign from json

$ g++ ttt.cc -std=c++11
ttt.cc: In function ‘int main()’:
ttt.cc:34:17: error: cannot convert ‘json {aka nlohmann::basic_json<>}’ to ‘Foo*’ in initialization
         Foo *f2=j;
                 ^

The full code is here, my environment is Ubuntu 16.04 + gcc 5.4.0

#include "json.hpp"

using json = nlohmann::json;

class Foo
{
public:
void bar() {
        std::cout << "bar()" << std::endl;
}
};

namespace nlohmann {
// You can also fully specialize on Foo*
template <typename T>
struct adl_serializer<T*> {
        static void to_json(json& j, const T* f) {
                j=reinterpret_cast<uintptr_t>(f);
        }
        static void from_json(const json& j, T*& f) {
                f=reinterpret_cast<T*>(j.get<uintptr_t>());
        }
};
}


int main()
{
        Foo f;
        f.bar();

        json j=&f;

        Foo *f2=j;
}

@theodelrieu
Copy link
Contributor

Oh, I think I understand, there is a method get_ptr that gives access to the internal JSON value, and calling get with a pointer type simply delegates the call to get_ptr...

This could be changed, it would break existing code though, I don't know what @nlohmann thinks about this.

Personally, I think we should change the current behaviour, it made sense before that conversion mechanism was introduced.

@intijk
Copy link
Author

intijk commented Jul 8, 2017

I see. So the serializer feature for now is not able to deal with pointer type due to this design.

I am not familiar with the design of it, but as a user, from my perspective the ability to serialize a pointer is kind of eagerly needed :)

@theodelrieu
Copy link
Contributor

Right, you could use a simple wrapper type to bypass that, while waiting for a patch, if we decide to patch it.

@intijk
Copy link
Author

intijk commented Jul 8, 2017

Yes, that is my plan, thanks.

@nlohmann
Copy link
Owner

nlohmann commented Jul 9, 2017

What exactly would need to be changed?

@theodelrieu
Copy link
Contributor

theodelrieu commented Jul 9, 2017

It would require this get overload to be removed:

// line 3651
template<typename PointerType, typename std::enable_if<
                 std::is_pointer<PointerType>::value, int>::type = 0>
    PointerType get() noexcept
    {
        // delegate the call to get_ptr
        return get_ptr<PointerType>();
    }

@nlohmann
Copy link
Owner

nlohmann commented Jul 9, 2017

Hm. This indeed breaks existing code. Of course there is the way to call get_ptr explicitly, but that's rather an excuse than a solution. And I am not convinced that this change should be made to store pointers as numbers inside a JSON value... Or am I missing potential benefits?

@theodelrieu
Copy link
Contributor

The point is just to let users handle pointers as they see fit, the library would not have a default behaviour in this case.

I'm not 100% sure we should make this change either, but I still think it's a good idea.

For users relying on get<string*>, it will fail to compile (and not silently compile and crash one day).
And as I said, the current behavior was there long before user-defined conversions, but now it just seems like an arbitrary restriction on which types could be converted with from/to_json.

@nlohmann
Copy link
Owner

nlohmann commented Jul 9, 2017

I reopen this and hope to get more input in the discussion. I currently do not see the point at all to store pointers inside JSON, so I don't really care about an elegant interface for this.

@nlohmann nlohmann reopened this Jul 9, 2017
@nlohmann nlohmann added kind: enhancement/improvement state: please discuss please discuss the issue or vote for your favorite option labels Jul 9, 2017
@nlohmann
Copy link
Owner

As there is no more discussion. I close this issue for now. Breaking code without an explicit benefit seems to be a bad idea.

@simon-winter
Copy link

Hey,
i am trying to convert a map of <string,BaseClass*> in and out of json. This map is inside another object, so i want to use the automatic serialization from maps with json. Aparently this library seems not to be able to resolve a map containing pointers and gives a compile error.

My goal is to parse this map as member variable, where the BaseClass* represents an interface with virtual methods and i plug in different deriving types in there.
in my toJson() i have a working workaround, but this way i loose the map structure and have instead a vector structure:

vector<string> mapEntries;					
for (auto var : nodes)
{
        json jsn = { var.first , *var.second };	
	mapEntries.push_back(jsn.dump());
}
return json{
	{"nodes", mapEntries}
};

any idea how to make json resolve my pointers in an stl, before parsing them?

@simon-winter
Copy link

simon-winter commented Jun 18, 2021

edit: i had the idea, that i can overwrite the function which uses exactly this map like:

void to_json(json& j, const map<string, BaseMessageVariable*>& p) {
    for (auto var : p)
    {
	j += { var.first, * var.second };
    }
}

but in output my json looks like an array not like a map ( i want to get rid of the squared brackets)
current:

"nodes": [
    ["testObjName",{"description":"desc","maxValue":34.0}],
    ["testObjName2",{"description":"","maxValue":0.0,}]
]

what i actually want:

"nodes": {
    "testObjName":{"description":"desc","maxValue":34.0},
    "testObjName2":{"description":"","maxValue":0.0}
}

@simon-winter
Copy link

found the solution!

void to_json(json& j, const map<string, BaseMessageVariable*>& p) {	
		json res;
		for (auto var : p)		{
			
			res.update(json{ { var.first, *var.second } });
		}
		j = res;
	}

the trick is to understand the differnece between array and object for json.
wrapping the json in two brackets makes it an object, then i use update to merge the json object into on e big object, which i then return.
the method is the standard type overwriting with global functions provided by the lib.
if this pointer dereferncing would be used autmatically, that woul dbe nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement/improvement kind: question state: please discuss please discuss the issue or vote for your favorite option
Projects
None yet
Development

No branches or pull requests

6 participants