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

Treat nullable fields as optional #1196

Closed
tyczj opened this issue Nov 12, 2020 · 10 comments
Closed

Treat nullable fields as optional #1196

tyczj opened this issue Nov 12, 2020 · 10 comments
Labels

Comments

@tyczj
Copy link

tyczj commented Nov 12, 2020

Currently if you have a data class that has nullable fields the serializer still considers the fields as required so for example

@Serializable
data class ResponseData(
    val id: Long
    val email: String?,
    val phone: String?,
    val text: String
}

I would have expected the nullable fields to be null if they didnt exist in the json response but they throw the MissingFieldException

To fix the error you just set the default value to null

@Serializable
data class ResponseData(
    val id: Long
    val email: String? = null,
    val phone: String? = null,
    val text: String
}

To prevent from having to go through every nullable field can we just have it so that if its nullable and no default value set then its also considered optional?

What is the point of even having nullable fields if you have to set a default value anyway (aside from being able to have a null value for the field) for it to be null if the fields does not exist in the json response?

It seems to me something like this would make more sense and be more kotlin like

@Serializable
data class ResponseData(
    val id: Long //Required because its not nullable
    val email: String?, //Optional
    val phone: String? = "123456789", //Optional thats nullable
    val text: String //Required because its not nullable
}
@tyczj tyczj added the feature label Nov 12, 2020
@psteiger
Copy link

psteiger commented Nov 12, 2020

I don't agree. In Kotlin, if you try to instantiate your first data class example in code, you will have to provide a value for the nullable fields, for they don't have a default value. So the notion of nullable == optional is not really a Kotlin idiom. Same goes for serialization.

We need a way to differentiate

{value: null}

and

{}

Sometimes, they don't mean the same thing. Both deserialize to:

data class(val value: Int? = null)

but only the former deserializes to:

data class(val value: Int?)

In other words, "undefined" is not always meant to mean the same as "defined as null".

@pdvrieze
Copy link
Contributor

It is possible for formats to support this independently (as in the XML format). But doing so does require the format itself to track the properties set and then generated synthetic property values for the values not set. Doing this is additional work and the way optional values works is better, but hidden from the format. On the other hand handling optional values is done by each serializer and thus depends on the Kotlin version used to compile the serializer (the serialization plugin), not the runtime library.

As @psteiger points out, for some formats (like JSon) there is a difference between an unset property and one with null value. In XML there is no notion of null value, so absence will have to do. The format therefore would need to make the decision. Having the format interact/reconfigure the serializer would put quite some requirements upon the serializer and therefore overkill.

@tyczj
Copy link
Author

tyczj commented Nov 12, 2020

Fair point, I am in the process of converting my serialization over from Moshi and it was able to do this so when I started getting this error in all my data classes I couldn't understand why missing properties were not just null since they were nullable

@ArcticLampyrid
Copy link

Actually nullable properties can have non-null default values.
For example, we have a config called WebUI, with the default value localhost:8888. NULL means not serving WebUI.
Requirement to set default to NULL explicitly makes it less confusing.

@sandwwraith
Copy link
Member

As it was correctly mentioned, missing property and null value are different things, so this likely wouldn't be implemented.

@tujhex
Copy link

tujhex commented Jan 15, 2021

Hi! I read this thread, and it looks strange for me - you say, that with default value in constructor field is optional, otherwise - required. Also we have this section of readme. It says, that exists @required annotation, which makes optional fields required...
So I looked what was before @required - and it was @optional annotation, which explicitly defined that annotated field is optional. (I think it's more obvious behavior, rather than current). But I think that's not the best choice - to revert.

@psteiger, your argument is that language constructor behaviour defines optionality of the fields - but I can argue with that, because we discuss fields, not constructors, and in case of fields optionality is nullability. (in many cases kotlin nullability is proposed as replacement of java optional)

So, that what I think about it. We have @required now, so maybe we can remove default values as requirements for optionality behavior and define it as just ? ? If we want specific values as defaults - we can define them, and if it needs to be required and nullable - we have @required. It seems as most obvious behaviour for me

@milosmns
Copy link

@tujhex you tagged @required and @optional, the GitHub accounts 😬

@kernitus
Copy link

Well, maybe we could have an option to enable this behaviour, like with ignoreUnknownKeys?

@sandwwraith
Copy link
Member

@kernitus That's what the new flag explicitNulls (#1535 ) does

@kernitus
Copy link

Ah nice!

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

No branches or pull requests

8 participants