Stiffstream / json_dto

A small header-only library for converting data between json representation and c++ structs
BSD 3-Clause "New" or "Revised" License
149 stars 18 forks source link

std::map with int keys fails at runtime #9

Closed DaDuFan closed 4 years ago

DaDuFan commented 4 years ago

First, thanks for this great library.

The implementation of map_like_associative_container does work only with string-keys, so a std::map with an int key fails at runtime, because object.AddMember( key, value, allocator ); asserts that key is a string.

e.g.:

struct test {
    std::map<int, int> map1;
    std::map<std::string, int> map2;

    template<typename Json_Io>
    void json_io(Json_Io & io)
    {
        io & json_dto::mandatory("map1", map1) 
        & json_dto::mandatory("map2", map2) ;
    }
};
test r2;
r2.map1[99] = 100;
r2.map1[199] = 9100;
r2.map2["99"] = 1100;
r2.map2["199"] = 19100;

std::string json = json_dto::to_json(r2);

As I needed a fast solution, I made a quick hack using std::to_string

https://github.com/DaDuFan/json_dto/commit/0158c4a359afb09cc7bb3ece312f04731d055e64

This hack is not good enough to be published, therefore I made no Pull request, but maybe you can use it as an inspiration for a better solution.

eao197 commented 4 years ago

Hi! Thanks for reporting this.

I am afraid this issue has no simple and quick solution. It's because there is a nature of JSON where keys should be represented as quoted strings. And it's a big question to me is it good for json-dto to hide that nature from a user. We have do discuss this issue inside our team.

As a workaround, I can propose to use a special wrapper for non-string keys. Something like:

template<typename T>
struct key_wrapper {
  T v_;
   ... // Constructors
   bool operator==(const key_wrapper & other) const noexcept { return v_ == other.v_; }
   bool operator<(const key_wrapper & other) const noexcept { return v_ < other.v_; }
   ...
   operator T() const { return v_; }
   ...
};

template<typename T>
void read_json_value(
    key_wrapper<T> & value,
    const rapidjson::Value & from)
{
   ... // Read a string and convert it to T.
}

template<typename T>
void write_json_value(
    const key_wrapper<T> & value,
    rapidjson::Value & object,
    rapidjson::MemoryPoolAllocator<> & allocator)
{
   ... // Convert value into a string and store it into object.
}

In that case, you can use std::map<key_wrapper<int>, int> instead of std::map<int, int>.

eao197 commented 4 years ago

Hi, @DaDuFan!

There is the following idea.

We can add a couple of new types to json-dto:

template<typename T>
struct const_associative_container_key {
  const T & v;
};
template<typename T>
struct mutable_associative_container_key {
  T & v;
};

There will be a couple of new versions of write_json_value and read_json_value for those new types:

template<typename T>
void write_json_value(
    const_associative_container_key<T> what,
    rapidjson::Value & object,
    rapidjson::MemoryPoolAllocator<> & allocator)
{
  // Just use write_json_value for T.
  write_json_value(what.v, object, allocator);
}

template<typename T>
void read_json_value(
    mutable_associative_container_key<T> what,
    const rapidjson::Value & from)
{
  // Just use write_json_value for T.
  read_json_value(what.v, from);
}

The code of read_json_value/write_json_value for associative containers will be modified this way:

---     read_json_value( key, it->name );
+++     read_json_value( mutable_associative_container_key{key}, it->name );
        read_json_value( value, it->value );

---             write_json_value( kv.first, key, allocator );
+++             write_json_value( const_associative_container_key{kv.first}, key, allocator );
                write_json_value( kv.second, value, allocator );

So in such an approach, you will have a possibility to place new overloads into json_dto namespace:

// Somewhere in your source code:
namespace json_dto {

inline void write_json_value(
    const_associative_container_key<int> what,
    rapidjson::Value & object,
    rapidjson::MemoryPoolAllocator<> & allocator)
{
  write_json_value(std::to_string(what.v), object, allocator);
}

inline void read_json_value(
    mutable_associative_container_key<int> what,
    const rapidjson::Value & from)
{
  std::string str_v;
  read_json_value(v, from);
  what.v = std::atoi(str_v.c_str());
}

}

If you like that approach we can update json-dto in a couple of days.

DaDuFan commented 4 years ago

Hi @eao197 ,

sorry for my late reply, and thanks for your fast and constructive reply.

Your idea with the new types sound great, and I think it would be a great improvement for json_dto. It is non intrusive and allows to easily handle int-keys.

If you implement it, I can guarantee that at least I will use it :-)

eao197 commented 4 years ago

It seems there is another flaw in json-dto support for custom formatters (introduced in 0.2.10 in the form of Reader_Writers): in the case of custom Reader_Writer and container-like fields the Reader_Writer will be applied to the whole container, not to items of the container.

I hope to address both of those issues in one json-dto's update. And now I'm searching for a way to do that :)

eao197 commented 4 years ago

Hi, @DaDuFan !

An updated version of json-dto is in the master branch. Changes are described in README.

There are also two new examples. One shows the simplest approach with overloading read/write_json_value functions. Another shows more complex case where two std::map<uint32_t, int> have to be represented differenly.

If there won't be any surprises I'll make a release of v.0.2.11 tomorrow.

DaDuFan commented 4 years ago

Sounds great. Thank you very much!

I will start using it in the next days.

DaDuFan commented 4 years ago

Hi @eao197 ,

just wanted to say thanks, again!

I'm using the release v.0.2.11 now.