can to_json() be defined inside a class?

[cwreynolds]

I have to_json() functions defined for classes in the global namespace, and for classes inside my own namespaces. Today I tried to make one for a class defined inside another class. Should that be possible? I'm pretty fuzzy on the details of how ADL works. I tried defining to_json() as a static method in the scope of the outer class, with a second argument of the inner class. I kept getting ‘Static_assert failed "could not find to_json() method in T's namespace" ’ until I moved the inner class (and its to_json()) out into the global namespace. I can live with that, but the inner class is used only within the outer class and seemed like it should be defined there.

class Outer
{
public:
    // ...
    class Inner
    {
        // ...
    };
    static void to_json(nlohmann::json& json_document, const Inner& i)
    {
        // ...
    }
    // ...
};

[jaredgrubb]

ADL will look in the namespace of the class, but nested classes don't count as "namespaces".

If you change "static" to "friend", that would allow you to define it "inside" the class but it's actually being declared in the surrounding namespace. This is kind of cheating to get what you want, and "friend" isn't really meant to be used for this purpose, but it would work. Note that the function is still in the namespace, not a member of Outer or Inner.

[jaredgrubb]

I do think there's an interesting question for @nlohmann here though .. should a member function named "to_json" be allowed by the library?

As a corrolary, range-based for loops will look for either a "begin/end" member function (like std::vector::begin) but will also look for an ADL-style "begin/end" function.

@cwreynolds : is that sort of what you were asking, or am I twisting your request a bit?

[nlohmann]

@jaredgrubb A member function would be nice, but I'd like to hear @theodelrieu 's opinion about this. He implemented all the ADL magic...

[cwreynolds]

Thanks for looking at this.

@jaredgrubb : mostly I wanted to make sure that the thing I tried to do does not work in the current implementation. Working around it, whatever that requires, is OK for me.

I will note that I use virtual polymorphic methods to do the actual work, my to_json functions are all like this:

inline void to_json(nlohmann::json& j, const MyDerivedClass& mdc)
{
    mdc.serializeToJson(j);
}

This allows me to define serializeToJson() methods on, say, MyDerivedClass and MyBaseClass, so I can have the MyBaseClass::serializeToJson() handle the data that is defined in the base class and MyDerivedClass::serializeToJson() explicitly call the base class method, plus handle the additional data for the derived class.

[rmstyrczula]

@cwreynolds You're still able to use polymorphism in non-member to_json functions by using dynamic_cast on the derived to the superclass.

Here's a small example.

[cwreynolds]

@rmstyrczula
Yes, that makes sense. But my point was not that I was forced to use methods, but rather that was my stylistic preference. I think that is what @jaredgrubb was asking in his question to me. My code is nearly all methods. One notable exception are the to_json functions. In my code I think it would feel more natural if to_json could be defined as a method on the relevant class.

(Perhaps worth noting, my original issue was about something else: a class and its to_json function appearing within another class. The discussion has gone in a different direction—toward a feature I would appreciate if it existed—but that was not the original issue I raised.)

[theodelrieu]

Sorry for being late on this one.

@cwreynolds To answer the original issue, if the inner class is public, you can simply define:

class Outer {
  public: class Inner{};
};

void to_json(json&, Outer::Inner const&) {}

If it is private, friend is the solution.

About supporting member functions by default, I'm quite opposed to it, mainly for the library code's maintainability. Also, what should the library do if both free function and member function are present?

Handling priorities with SFINAE is quite hard (you can look at how we handle json.get<T>() for non-copyable types for example).

However, it is possible to write your own JSON serializer, which would call a member-function to_json when detected, and the ADL version when not. This is the lowest-level customization point the library provides, it is thus quite hard to write.

[nlohmann]

Thanks @theodelrieu for clarifying. I have nothing to add.

@cwreynolds Do you need further assistance?

[cwreynolds]

@nlohmann:

No. As I mentioned in my original comment, I had already “worked around” the issue before I brought it up here. Thanks to everyone who provided thoughtful comments.

I still think “...it would feel more natural if to_json could be defined as a method on the relevant class...” but I do not expect my personal stylistic preferences to drive the design of this excellent library, and certainly not impede its ability to deal with all legal C++ code.

[nlohmann]

@cwreynolds The problem is that the current approach already uses more magic than I can possibly understand ;-)