search

mgholam / fastJSON

Smallest, fastest polymorphic JSON serializer
https://www.codeproject.com/Articles/159450/fastJSON-Smallest-Fastest-Polymorphic-JSON-Seriali
MIT License
479 stars 147 forks source link

JSONParameters.DateTimeMilliseconds not necessary #98

Closed xmedeko closed 3 years ago

xmedeko commented 5 years ago

JSONParameters.DateTimeMilliseconds is not necessary. Just serialize the ms only if dt.Millisecond > 0 (in JSONSerializer.WriteDateTime)

mgholam commented 5 years ago

It's for those who don't want milliseconds in the string regardless of the actual value.

xmedeko commented 5 years ago

IMO serialization/deserialization should produce same value, or the closest value to the original as possible.

If someone does not need ms then he should not make DateTime with ms. Or, if he has DateTime.Now then it's possible to make a function to clamp DateTime to ms. Or at least, the default value should be DateTimeMilliseconds = true.

mgholam commented 5 years ago

You can set the defaults to what ever you require before using fastJSON, if I change this it will break existing users code.

xmedeko commented 5 years ago

Yes, breaking BC is a problem always. But I think this feature is not well chosen. It was a bas surprise for me that DateTime serialization/deserialization changes the value. I think it's not necessary to change the behaviour now, but maybe with some next bigger release. And it would be nice to serialize whole value, i.e. with nanos, too.

xmedeko commented 5 years ago

Also see PR #3