Add UtcDateTypeAdapter for Gson #2549
Conversation
There was a problem hiding this comment.
Hey, I think I understand the original issue but I'm still confused, sorry:
- We do normally serialize dates as Long in the DB. Where do we have dates otherwise? In polls?
- Why do we need to embed ISO date parser? I understand why default date format doesn't work but doesn't it work if we use it directly?
About encoding libraries, did you consider kotlinx.serialization? It generates more code but should be the fastest. I also like Moshi because, well, square and Jack
| default: | ||
| String date = in.nextString(); | ||
| // Instead of using iso8601Format.parse(value), we use Jackson's date parsing | ||
| // This is because Android doesn't support XXX because it is JDK 1.6 |
There was a problem hiding this comment.
doesn't support what? Android is much higher than JDK 1.6 too?
There was a problem hiding this comment.
(This is ancient code I copied from Gson)
X is the format string placeholder for timezone, see here https://docs.oracle.com/javase/10/docs/api/java/text/SimpleDateFormat.html
There was a problem hiding this comment.
Should I try improving this code? Or do the Moshi migration, it will make it obsolete
There was a problem hiding this comment.
What are your thoughts on kotlinx?
I honestly don't know. Here's how Moshi does it:
https://github.com/square/moshi/blob/484d525db44f7d7b1c5a6f6f30008b12f405e1e4/moshi-adapters/src/main/java/com/squareup/moshi/adapters/Iso8601Utils.kt#L79
maybe keep it as is, just comment on the class or where it's used why we need it.
There was a problem hiding this comment.
I had a look at kotlinx.serialization and it seems awesome, but maybe more for serverside. For our usecase I see two problems
- no first-party support for Retrofit
- no support for java.util.Date, so we need to either write our own adapter again or use another date framework
There was a problem hiding this comment.
I used it on the client but with another network framework and yes, no date adapter kind of kills it.
Let's just go with what we have here (or take Kotlin version from Moshi, whatever you prefer)
There was a problem hiding this comment.
I think I'll merge it like it is now so we get the crash fixed
Basically the built-in
Datehandling sucks, see google/gson#935closes #2476
I was thinking we could also migrate to Moshi, but
➕ Moshi has better Kotlin support
➖ According to my measurements here it is slower than Gson
➖ Migration is quite an effort and we could break something