Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow null for std::optional #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

omartijn
Copy link

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
Copy link
Member

eao197 commented Jan 17, 2024

@ngrodzitski , what do you think?

@omartijn
Copy link
Author

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

@eao197
Copy link
Member

eao197 commented Jan 23, 2024

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:

  • 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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants