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

Allow null for `std::optional` #22

Open omartijn opened 10 months ago

omartijn commented 10 months ago

This ensures the same behaviour for std::optional that we get for json_dto::nullable_t: It won't fail when feeding it a null value. I think this makes sense, as json_dto::nullable_t is documented as being an "alternative for std::optional".

eao197 commented 10 months ago

@ngrodzitski , what do you think?

omartijn commented 9 months ago

I'm thinking maybe we could even make json_dto::nullable_t an alias for std::optional if that's available.

eao197 commented 9 months ago

Hi! I'm sorry for long reaction time.

If I remember correctly the motivation behind json_dto::nullable_t was to provide a type that can be represented by null in JSON. For example:

struct demo {
  json_dto::nullable_t<int> x;
  ...
};

If demo::x has no value then it has to be serialized as "x":null to JSON. Contrary, if JSON contains "x":null it will be read and demo::x will be set to null.

The support for std::optional is a bit different. From my point of view, std::optional<int> represent the following cases:

But "x":null for std::optional<int> can't be treated as std::nullopt from my point of view, because "x" is present (it means that it isn't optional) and has a special null value that is not a valid int, but is still here and should be treated accordingly.

So in a case of std::optional<int> we have just two choices during deserialization:

But in a case of json_dto::nullable_t<int> we have three cases:

If we allow null for std::optional then we may lost a case when "x":null (when this case has to be handled separately).

So now I'm in doubt we can accept your PR. But maybe there is another opinion? @ngrodzitski, do you have one?