tgockel / json-voorhees

A killer modern C++ library for interacting with JSON.
http://tgockel.github.io/json-voorhees/
Apache License 2.0
129 stars 18 forks source link

Remove optional<T> specialization of container_adapter #155

Closed tgockel closed 4 years ago

tgockel commented 4 years ago

This specialization is confusing.


In the 1.x line, there was a container_adapter<T> parial specialization for optional<T> which was meant to automatically capture optional-like containers.

struct my_thing
{
    jsonv::optional<std::string> a;
};

jsonv::formats get_json_formats_for_my_thing()
{
    return jsonv::formats_builder()
        .type<my_thing>
            .member("a", &my_thing::a)
        .register_container<jsonv::optional<std::string>>()
        ;
}

The idea was that the JSON string { "a": null } would automatically be deserialized to have my_thing::a properly end up as nullopt. If you think of optional<T> as a container of either 0 or 1 values, this makes a lot of sense. However, there are a few important distinctions between a "regular" container (think vector or list) and optional.

  1. How should null be interpreted? For a classic container, null is considered illegal -- the empty list ([]) is the default (or non-presense). For optional, null seems a reasonable representation for empty (nullopt), while an empty list [] would be unfitting.
  2. How should types with null as a valid representation look? This is a dumb argument...it still applies for the change to.

Since the specialization lived in serialization_optional.hpp, anyone relying on the old behavior will get a reasonably-accessable compilation error during upgrade. Those deleting their #include <jsonv/serialization_optional.hpp> without further thought will get a less reasonable compilation error in the internals of container_adapter.