Maps with enum class keys which are convertible to JSON strings should be converted to JSON dictionaries

[ghost]

The following code:

#include <fstream>
#include <iomanip>
#include <iostream>
#include <unordered_map>
#include <nlohmann/json.hpp>

using json = nlohmann::json;

enum class Species {
	DOG,
	OCTOPUS
};

void from_json(const json& j, Species& sp) {
	std::string s = j;
	if (s == "DOG") {
		sp = Species::DOG;
	} else if (s == "OCTOPUS") {
		sp = Species::OCTOPUS;
	} else {
		throw std::invalid_argument("Unknown species");
	}
}

void to_json(json& j, const Species sp) {
	if (sp == Species::DOG) {
		j = "DOG";
	} else {
		j = "OCTOPUS";
	}
}

int main() {
	json j;
	std::unordered_map<Species, bool> is_aquatic;
	is_aquatic[Species::DOG] = false;
	is_aquatic[Species::OCTOPUS] = true;
	j = is_aquatic;
	std::cout << j << std::endl;
}

produces the output [["DOG",false],["OCTOPUS",true]], which is not what the user would expect IMHO. Would it be possible to render this map as {"DOG":false,"OCTOPUS":true}?

[theodelrieu]

I'll take a look at it tomorrow

[nlohmann]

As information: std::map and std::unordered_map are created as JSON objects if the keys can be converted to a string. Otherwise, an array of pairs is created.

In this example, there is "only" a Species -> json conversion, so maybe this case was just forgotten. But before I start wondering whether we could detect whether we can make a string out of that json, I rather wait for @theodelrieu 's idea on this.

[RokerHRO]

I see problems when there are enum values that are json-ized into non-strings:

enum class Species {
	DOG,
	OCTOPUS,
        LIGER,
        TIGON
};

void to_json(json& j, const Species sp) {
	switch(sp) {
             case Species::DOG:
		j = "DOG";
                break;
             case Species::OCTOPUS:
                 j = "OCTOPUS";
                break;
            case Species:LIGER:
                j = std::vector<string>({"LION", "TIGER"});
                break;
            case Species:TIGON:
                j = std::vector<string>({"TIGER", "LION"});
                break;
	}
}

[theodelrieu]

@RokerHRO Indeed, what can be done is the following:

If the map/unordered_map/etc... key_type is convertible to json, then convert it, and if it's not a string type, create an array of JSON values instead of an object.

This becomes quite confusing, but I think it still makes sense.

[RokerHRO]

@theodelrieu : so what shall be done when a map contains both?

std::map<Species,int> bogus = {{DOG, 4}, {OCTOPUS,1}, {LIGER:42}};

some keys are convertible to strings, others are not. So this map would become a JSON object when all keys are (convertible to) strings, and when at least one element's key is not convertible to string the whole thing is JSON-ized as array-of-pairs? I'd say, that would be very confusing! Please, don't do that! So I hope I have misunderstand you. ;-)

[theodelrieu]

I agree this is quite terrible, and even if having such hybrid representation of the same type looks bad in the first place, there is no way to enforce that every enum value is converted to a string at compile time.

Therefore I think you should instead partially specialize adl_serializer for this use case:

namespace nlohmann {
template <typename T>
struct adl_serializer<std::map<Species, T>> {
    using Map = std::map<Species, T>;  

    static void from_json(json const& j, Map& m) { /* ... */ }
    static void to_json(json& j, Map const& m) { /* ... */ }
};
}

[ghost]

OK, OK :) Generating such JSON would indeed be quite confusing. But could we have the parser be able to deserialise the input {"DOG":false,"OCTOPUS":true} into std::map<Species, bool> if Species can be deserialised from JSON strings? This would not be confusing, would it?

Partially specialising adl_serializer is not great because I'll have to do it for every enum class separately, IMHO.

[ghost]

PS. When serialising std::unordered_map into an array, what determines the ordering of entries?

[theodelrieu]

@katastrofa999

[...] if Species can be deserialised from JSON strings

Same problem here, how can we enforce that at compile-time? The only thing there is here is the from_json method, which could convert from anything, depending on how you implement it.

Partially specialising adl_serializer is not great because I'll have to do it for every enum class separately, IMHO.

Not necessarily:

namespace nlohmann {

template <typename T, typename = std::enable_if_t<is_enum_class<T>::value>>
struct adl_serializer<...> { /* Same code as above */ };
}

The is_enum_class trait is a conjunction of std::is_enum<T>::value and not std::is_convertible<T, std::underlying_type_t<T>>::value.

[theodelrieu]

@katastrofa999

When serialising std::unordered_map into an array, what determines the ordering of entries?

We iterate on the object with begin()/end(), so it depends on the actual container.

[ghost]

Same problem here, how can we enforce that at compile-time?

I meant something like that:

json j  = "{\"DOG\":false,\"OCTOPUS\":true}"_json;
std::unordered_map<Species, bool> = j; // if the input can be parsed into this type, great. If not, throw an exception.

I mean, you can't generally enforce the success of deserialisation at compile time, because it depends on inputs which you know only at runtime...

We iterate on the object with begin()/end(), so it depend on the actual container.

That's not terribly great for std::unordered_map and std::unordered_set, is it? A JSON array implies that order matters, but the type deserialised into it is unordered. It makes unit and regression testing tricky.

[RokerHRO]

@katastrofa999: I don't like that if the conversion is not roundtrip-safe. So your JSON object
{"DOG": false, "OCTOPUS": false} can be parsed into a map<Species,bool>, but when someone adds a LIGER to that map its serialization is no longer a JSON object but a JSON array. That would create a lot of WTF moments in the users' brains, for sure.

[ghost]

Could this be optional behaviour, then? If one uses JSON files to store user-editable application configuration, then writing maps by hand as JSON objects is more intuitive than writing them as JSON arrays. Usually, you're not going to change the configuration after loading it from JSON, or serialise it back to JSON.

If this behaviour is optional, you can add a check that the serialisation fails at runtime if the map cannot be serialised as JSON object, without affecting the rest of the code which does not use this option.

[theodelrieu]

I mean, you can't generally enforce the success of deserialisation at compile time

Sorry for the confusion, I mean that the library performs a list of compile-time checks to decide which overload it has to use (e.g. if std::is_convertible<Key, std::string>::value == true then choose the overload which deals with JSON objects, etc...).

In this specific case, your enum is not convertible to string (i.e. std::is_convertible returns false).

It makes unit and regression testing tricky.

I agree, but it's related to using unordered containers in the first place, I don't see how the library could guess the correct order when serializing those.

[theodelrieu]

Could this be optional behaviour, then?

Introducing specific cases in the default-provided conversions is tricky, I'd like to avoid such complexity in the library's code (which is already quite complex...).

For those specific cases, I'd recommand the solution proposed above, with partial specializations. It will not harm other users of the library, and will fulfill your use-case (even if it can be painful to implement, but we can help you having a robust implementation).

[ghost]

Thanks, I'll take a look at that.