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

enum to json mapping #1208

Closed
dgendreau opened this issue Aug 22, 2018 · 30 comments · Fixed by #1323
Closed

enum to json mapping #1208

dgendreau opened this issue Aug 22, 2018 · 30 comments · Fixed by #1323
Assignees
Labels
kind: enhancement/improvement release item: ✨ new feature solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@dgendreau
Copy link

dgendreau commented Aug 22, 2018

One use-case I run into a lot is mapping enums to serialize as non-integers such as strings. I know its not really modern C++ to use preprocessor macros, but I wrote one that declares a from_json()/to_json() specialization in one statement to make it easier and less redundant to specialize the mapping between a given set of enum values and a set of json values. Motivating example:

// enum type declaration
enum TaskState {
    TS_STOPPED,
    TS_RUNNING,
    TS_COMPLETED,
    TS_INVALID=-1,
};

JSON_SERIALIZE_ENUM( TaskState, {
    {TS_INVALID, nullptr},
    {TS_STOPPED, "stopped"},
    {TS_RUNNING, "running"},
    {TS_COMPLETED, "completed"},
});

// Usage:
// enum -> json string
nlohmann::json j = TS_STOPPED;
assert(j == "stopped");

// json string-> enum
nlohmann::json j3 = "running";
assert( j3.get<TaskState>() == TS_RUNNING);

// unmapped json -> enum (default value is first pair)
nlohmann::json jPi = 3.14;
assert( jPi.get<TaskState>() == TS_INVALID );

The JSON_SERIALIZE_ENUM() macro above is implemented as:

#define JSON_SERIALIZE_ENUM( ENUM_TYPE, ...)\
	inline void to_json( nlohmann::json& j, const ENUM_TYPE& e){\
		static const nlohmann::multi_bimap<ENUM_TYPE,nlohmann::json> m=__VA_ARGS__;\
		j = m.at_right(e);\
	} \
	inline void from_json( const nlohmann::json& j, ENUM_TYPE& e){\
		static const nlohmann::multi_bimap<ENUM_TYPE,nlohmann::json> m=__VA_ARGS__;\
		e = m.at_left(j);\
	}

This example is still just a sketch since it does not take into account that basic_json can be specialized for different types.

One benefit of the multi_bimap<> is that it allows multiple enum values to map to the same JSON value and multiple JSON values to map to the same enum. The first matching pair found in at_right()/at_left is returned.

I can provide you with implementation details for the multi_bimap<TL,TR> class but it is mostly just a simple wrapper around a vector<pair<TL,TR>> that is linear searched in at_right()/at_left(). When a key is not found in at_right() or at_left(), the first pair is used as default values.

@nlohmann nlohmann added kind: enhancement/improvement state: please discuss please discuss the issue or vote for your favorite option labels Aug 23, 2018
@dgendreau
Copy link
Author

dgendreau commented Aug 24, 2018

Another option for the macro is to use the std::find_if() algorithm. This a little more wordy but would do away with the need for the multi_bimap class:

#define NLOHMANN_JSON_SERIALIZE_ENUM( ENUM_TYPE, ...)\
    inline void to_json( nlohmann::json& j, const ENUM_TYPE& e) {\
        static const std::vector<std::pair<ENUM_TYPE,nlohmann::json>> m=__VA_ARGS__;\
        auto it = std::find_if(m.cbegin(),m.cend(),[e](const auto& ej_pair)->bool{return ej_pair.first==e;});\
        j = ((it!=m.cend())?it:m.cbegin())->second;\
    }\
    inline void from_json( const nlohmann::json& j, ENUM_TYPE& e) {\
        static const std::vector<std::pair<ENUM_TYPE,nlohmann::json>> m=__VA_ARGS__;\
        auto it = std::find_if(m.cbegin(),m.cend(),[j](const auto& ej_pair)->bool{return ej_pair.second==j;});\
        e = ((it!=m.cend())?it:m.cbegin())->first;\
    }

@nlohmann
Copy link
Owner

I like the proposal from @dgendreau. Unless anyone objects, I would add it for the 3.2.1 release.

@theodelrieu - What do you think?

@theodelrieu
Copy link
Contributor

It looks good to me as well. I just have two remarks:

  • Generic lambdas are C++14, so you must type the whole type yet another time.
  • Passing ENUM_TYPE instead of const ENUM_TYPE& in to_json would be better.

Other than that, it's all good!

@nlohmann
Copy link
Owner

Thanks for the quick feedback!

@dgendreau
Copy link
Author

You might also want to have a second version of the macro that takes (ENUM_TYPE, JSON_TYPE, ...) to allow the user to map to something other than the default nlohmann::json type.

@theodelrieu
Copy link
Contributor

@dgendreau this can already be done in the macro, by templating on BasicJsonType

@dgendreau
Copy link
Author

I hadnt thought of that. Yes, thats a cleaner approach.

@dgendreau
Copy link
Author

dgendreau commented Sep 11, 2018

Thanks for the feedback @theodelrieu.
Is this version closer to what you had in mind?
Edit: CORRECTED

#define NLOHMANN_JSON_SERIALIZE_ENUM( ENUM_TYPE, ...)\
    template<typename BasicJsonType>\
    inline void to_json( BasicJsonType& j, ENUM_TYPE e) {\
        static_assert(std::is_enum<ENUM_TYPE>::value, #ENUM_TYPE " must be an enum!");\
        static const std::vector<std::pair<ENUM_TYPE,BasicJsonType>> m=__VA_ARGS__;\
        auto it = std::find_if(m.cbegin(),m.cend(),[e](const std::pair<ENUM_TYPE,BasicJsonType>& ej_pair)->bool{return ej_pair.first==e;});\
        j = ((it!=m.cend())?it:m.cbegin())->second;\
    }\
    template<typename BasicJsonType>\
    inline void from_json( const BasicJsonType& j, ENUM_TYPE& e) {\
        static_assert(std::is_enum<ENUM_TYPE>::value, #ENUM_TYPE " must be an enum!");\
        static const std::vector<std::pair<ENUM_TYPE,BasicJsonType>> m=__VA_ARGS__;\
        auto it = std::find_if(m.cbegin(),m.cend(),[j](const std::pair<ENUM_TYPE,BasicJsonType>& ej_pair)->bool{return ej_pair.second==j;});\
        e = ((it!=m.cend())?it:m.cbegin())->first;\
    }

@theodelrieu
Copy link
Contributor

No problem! That's it, but there's one nlohmann::json left.

@dgendreau
Copy link
Author

dgendreau commented Sep 11, 2018

Suggested user documentation for this feature:
Insert the following just before the "Binary formats (CBOR, MessagePack, and UBJSON)" section

Specializing enum conversion

By default, enum values are serialized to JSON as integers. In some cases this could result in undesired behavior. If an enum is modified or re-ordered after data has been serialized to JSON, the later de-serialized JSON data may be undefined or a different enum value than was originally intended.

It is possible to more precisely specify how a given enum is mapped to and from JSON as shown below:

// example enum type declaration
enum TaskState {
    TS_STOPPED,
    TS_RUNNING,
    TS_COMPLETED,
    TS_INVALID=-1,
};

// map TaskState values to JSON as strings
NLOHMANN_JSON_SERIALIZE_ENUM( TaskState, {
    {TS_INVALID, nullptr},
    {TS_STOPPED, "stopped"},
    {TS_RUNNING, "running"},
    {TS_COMPLETED, "completed"},
});

The NLOHMANN_JSON_SERIALIZE_ENUM() macro declares a set of to_json() / from_json() functions for type TaskState while avoiding repetition and boilerplate serilization code.

Usage:

// enum to JSON as string
json j = TS_STOPPED;
assert(j == "stopped");

// json string to enum
json j3 = "running";
assert( j3.get<TaskState>() == TS_RUNNING);

// undefined json value to enum (where the first map entry above is the default)
json jPi = 3.14;
assert( jPi.get<TaskState>() == TS_INVALID );

Just as in Arbitrary Type Conversions above,

  • NLOHMANN_JSON_SERIALIZE_ENUM() MUST be declared in your enum type's namespace (which can be the global namespace), or the library will not be able to locate it and it will default to integer serialization.
  • It MUST be available (e.g., proper headers must be included) everywhere you use the conversions.

Other Important points:

  • When using get<ENUM_TYPE>(), undefined JSON values will default to the first pair specified in your map. Select this default pair carefully.
  • If an enum or JSON value is specified more than once in your map, the first matching occurrence from the top of the map will be returned when converting to or from JSON.

@dgendreau
Copy link
Author

dgendreau commented Sep 11, 2018

Should we look into serializing third party enums with an adl_serializer<> specialization or is that overkill? I think that would require the to_json()/from_json() functions to also be declared static.

@theodelrieu
Copy link
Contributor

theodelrieu commented Sep 12, 2018

Should we look into serializing third party enums with an adl_serializer<> specialization

The library must not specialize adl_serializer, ever. Here is the current way conversions work:

  1. Constructor will call JSONSerializer<T>::to_json
  2. If JSONSerializer<T> (e.g. adl_serializer<T>) was specialized, if will take that specialization.
  3. If nlohmann::to_json is called by JSONSerializer<T>::to_json, the library will try to find a valid overload via ADL.
  4. If no viable overload is found via ADL, then it will look at overloads defined inside conversions/to_json.hpp.
  5. If none is found, you lose.

About 4, it's the only place where we can put types that are in a different namespace, because each overload is visible by nlohmann::to_json.

That's why specializations are reserved to users only.

Note: everything said here applies to from_json as well.

@theodelrieu
Copy link
Contributor

I'm not sure about the error handling here, but we cannot please everybody anyway...

One last thing that would be good: adding a static_assert(std::is_enum<ENUM_TYPE>::value, "").

@dgendreau
Copy link
Author

dgendreau commented Sep 12, 2018

Re: adl_serializer,
Understood. I was thinking of the macro as the user defining a from_json/to_json, not the library defining it. Thats why I was making sure we didnt have a missing scenario where the user needs to specialize the serialization of and enum in a namespace they do not control. Thats probably overkill for now.

Re: error handling,
Yes. I was torn between throwing an exception when an undefined enum or json value is serialized versus failing over to a reasonable default value.

Re: static_assert,
I updated the macro above and added a human readable message to it.

@theodelrieu
Copy link
Contributor

Could you add the static_assert in to_json as well?

I was looking at the new macro as the user defining a from_json/to_json, not the library defining it.

I misunderstood what you meant, it would be possible of course, but I'd rather leave specializations being explicit to write. For now at least.

@dgendreau
Copy link
Author

Done

@theodelrieu
Copy link
Contributor

Looks good to me, time to open a PR! :)

@dgendreau
Copy link
Author

ok. ill do that after work today.

@dgendreau
Copy link
Author

oh, what file would you recommend adding the macro to?

@theodelrieu
Copy link
Contributor

Good question... Either in json.hpp or in a new file that will get included by json.hpp.

@nlohmann
Copy link
Owner

@dgendreau Can you open a PR?

@dgendreau
Copy link
Author

Hi Niels,
Sorry I was delayed. I didnt want to submit a PR without verifying it first.
I added the new header and manually patched it into the amalgamated json.hpp and it works in practice, but I was stuck trying to set up cmake / amalgamate and run the unit tests. I'm on a widows MSVC 2015 build environment. Is there an easier way to run cmake / amalgamate / unit tests for a Windows / VC2015 machine?

@nlohmann
Copy link
Owner

For amalgamation, you do not need CMake, but only Python and astyle.

@nlohmann
Copy link
Owner

If you are still stuck on this, you can also send me a patch and I can create a PR.

@nlohmann
Copy link
Owner

nlohmann commented Oct 6, 2018

@dgendreau Can you send me a patch so I can make a PR?

@nlohmann
Copy link
Owner

@dgendreau Could you open a PR or send me a patch?

@nlohmann
Copy link
Owner

@dgendreau @theodelrieu Do we agree that #1208 (comment) should be added to the library? If so, I could create a PR.

@dgendreau
Copy link
Author

Hi. Sorry I was away. I'll post a PR this evening.

@nlohmann
Copy link
Owner

@dgendreau Any news on this? As I wrote above: I could also create a PR if we agree that #1208 (comment) should be the fix.

@nlohmann nlohmann added state: waiting for PR solution: proposed fix a fix for the issue has been proposed and waits for confirmation and removed state: please discuss please discuss the issue or vote for your favorite option state: waiting for PR labels Oct 25, 2018
@nlohmann nlohmann self-assigned this Oct 26, 2018
@nlohmann
Copy link
Owner

PR #1323 contains the macro definition, the documentation, and a few test cases. Thanks everybody for the good work! As I had some time, I opened the PR myself. It would be great if you could have a look whether anything else needs to be added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement/improvement release item: ✨ new feature solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants