Make makeNullable extension, make NullableSerializer internal #541
Conversation
qwwdfsad
commented
Aug 22, 2019
qwwdfsad
commented
runtime/commonMain/src/kotlinx/serialization/internal/NullableSerializer.kt
Outdated
Show resolved
Hide resolved
runtime/commonMain/src/kotlinx/serialization/internal/NullableSerializer.kt
Show resolved
Hide resolved
runtime/commonMain/src/kotlinx/serialization/internal/NullableSerializer.kt
Outdated
Show resolved
Hide resolved
runtime/commonMain/src/kotlinx/serialization/internal/NullableSerializer.kt
Outdated
Show resolved
Hide resolved
| */ | ||
| public val <T : Any> KSerializer<T>.nullable: KSerializer<T?> | ||
| get() { | ||
| return NullableSerializer(this) |
There was a problem hiding this comment.
One question. What happens if the receiver is already the NullableSerializer (or perhaps a custom serializer that handles nulls directly?). I don't think that nested NullableSerializers are correct for all formats (where notnullmark is actually used)
There was a problem hiding this comment.
Good point. Checking this.descriptor.isNullable will also help to save some allocations.
There was a problem hiding this comment.
@qwwdfsad Could you take a look at this?
There was a problem hiding this comment.
Fixed.
I don't think that nested NullableSerializers are correct for all formats (where notnullmark is actually used)
It should be, if you know the case where this property doesn't hold, please report
runtime/commonMain/src/kotlinx/serialization/internal/NullableSerializer.kt
Show resolved
Hide resolved
| */ | ||
| public val <T : Any> KSerializer<T>.nullable: KSerializer<T?> | ||
| get() { | ||
| return NullableSerializer(this) |
There was a problem hiding this comment.
@qwwdfsad Could you take a look at this?
* Introduced @InternalSerializationApi that provides no compatibility guarantees but is effectively public
* Extension is more expressive, readable and is aligned with Kotlin conventions
* NullableSerializer is an implementation detail and should not be exposed
qwwdfsad
force-pushed
the
nullable-ext
branch
from
ca1f770
to
4df8d10
Compare
