You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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".
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:
structdemo {
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:
if std::optional<int> has a value it will be serialized normally (for example: "x":1);
if std::optional<int> has no value it won't be serialized at all;
"x":1 is present in JSON, it will be deserialized as std::optional<int> with a value;
if there is no "x" in JSON, the std::optional<int> will be initialized with std::nullopt.
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:
value is present and is a valid int;
value is missed, so there is no value at all.
But in a case of json_dto::nullable_t<int> we have three cases:
value is present and is a normal int;
value is present, but is a null, it's not a normal int, but a special null value;
value is not present at all (it's not a int, and not the null). And this case can be handled too.
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?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This ensures the same behaviour for
std::optionalthat we get forjson_dto::nullable_t: It won't fail when feeding it a null value. I think this makes sense, asjson_dto::nullable_tis documented as being an "alternative for std::optional".