Serializers for common java types #350
Conversation
… polymorphic module builder and fixed old tests (working only in bootstrap mode :()
built not only with bootstrap
Introduce SubtypeNotRegisteredException Small testing utilities and refactoring of test data
Move test data to separate file
because Konan needs to be updated to be able to work with -dev-1984 plugin.
'todo: arrays' erased intentionally because we won't support polymorphic arrays – at least for now.
…te files. Some methods converted to extensions.
… implementation (except polymorphic, which is already tested indirectly)
# Conflicts: # build.gradle # gradle.properties # runtime/common/src/main/kotlin/kotlinx/serialization/Annotations.kt # runtime/common/src/test/kotlin/kotlinx/serialization/features/SkipDefaults.kt # runtime/js/src/main/kotlin/kotlinx/serialization/Serialization.kt # runtime/jvm/src/main/kotlin/kotlinx/serialization/Serialization.kt # runtime/native/src/main/kotlin/kotlinx/serialization/Platform.kt
runtime/jvm/src/main/kotlin/kotlinx/serialization/java/JavaTypesModule.kt
Outdated
Show resolved
Hide resolved
| import kotlin.reflect.* | ||
|
|
||
| /** | ||
| * Module with external serializers for the following Java types: |
There was a problem hiding this comment.
How are you going to combat this class from playing whack-a-mole forever in the future from people wanting their type included. For example, why isn't AtomicIntegerArray included? Why isn't java.time.*. Why isn't File?
There was a problem hiding this comment.
I don't mind including other types on a demand-driven basis when it is reasonable, an additional bonus is that this kind of features is a good "first-time contribution".
The only question is what is a "reasonable" request. E.g. java.sql.Date is a good candidate, while java.io.File is definitely not (at least because its representation is OS/machine/JVM specific). I know no strict criteria for a type to be included except common sense.
|
@qwwdfsad IMO we should move all this stuff to another module to publish separately, just like add-on formats or schema generators. People who are using the library in multiplatform context don't need all these |
| * Serializer for [java.util.Date] which serializers date as format string in default US locale. | ||
| */ | ||
| public object DateSerializer : KSerializer<Date> { | ||
| private val format = DateFormat.getDateTimeInstance(DateFormat.DEFAULT, DateFormat.DEFAULT, Locale.US)!! |
There was a problem hiding this comment.
Why a non standard format, without a time zone reference?
There was a problem hiding this comment.
And, there are actually a number of different relevant standards. You probably want to create serializers for each of them. As well as with or without time. Of course maybe dates/calendars/instants (whichever date type you want to use) are not the best starting point for this.
|
TODO: discuss FQN in descriptors |
| private val format = DateFormat.getDateTimeInstance(DateFormat.DEFAULT, DateFormat.DEFAULT, Locale.US)!! | ||
|
|
||
| override fun serialize(encoder: Encoder, obj: Date) { | ||
| val string = synchronized(format) { format.format(obj) } |
There was a problem hiding this comment.
Why you've preferred synchronized on a format instead of making it ThreadLocal?
sandwwraith
force-pushed
the
new_polymorphic
branch
2 times, most recently
from
cdcbf81
to
50e8af8
Compare
sandwwraith
force-pushed
the
dev
branch
2 times, most recently
from
5789e71
to
1efb642
Compare
I think so too. I have my own suggested additions as well (serializers to generate hashcode?). But that doesn't really belong in the main runtime support library. |
No description provided.