Add config option to save DateTime and TimeOffset as Int64 #127
Conversation
hymerman
force-pushed
the
dates-as-integers
branch
from
90acf3c to
0cbe3cb
Compare
| if (instance is DateTimeOffset) { | ||
| var dateTimeOffset = (DateTimeOffset)instance; | ||
| serialized = new fsData(dateTimeOffset.ToString(DateTimeOffsetFormatString)); | ||
| serialized = new fsData(dateTimeOffset.Ticks); |
There was a problem hiding this comment.
Is this change intentional? The fsConfig comment implies that this should still be serialized as a string.
There was a problem hiding this comment.
Hmm, I honestly can't remember - I wrote this ages ago and forgot to submit a PR... it looks like I thought I had a reason for sticking to string for DateTimeOffset. Maybe this code is just a left-over experiment? I'll have a look.
| if (instance is DateTime) { | ||
| var dateTime = (DateTime)instance; | ||
| serialized = new fsData(dateTime.ToString(DateTimeFormatString)); | ||
| if(Serializer.Config.SerializeDateTimeAsInteger) { |
| serialized = new fsData(dateTime.Ticks); | ||
| } else { | ||
| serialized = new fsData(dateTime.ToString(DateTimeFormatString)); | ||
| return fsResult.Success; |
There was a problem hiding this comment.
drop return (handled immediately below)
| if (instance is TimeSpan) { | ||
| var timeSpan = (TimeSpan)instance; | ||
| serialized = new fsData(timeSpan.ToString()); | ||
| if(Serializer.Config.SerializeDateTimeAsInteger) { |
| } | ||
|
|
||
| public override fsResult TryDeserialize(fsData data, ref object instance, Type storageType) { | ||
| if (data.IsString == false) { |
There was a problem hiding this comment.
I would drop all of the config checks in this function; deserialization should be successful regardless of the current config mode.
hymerman
force-pushed
the
dates-as-integers
branch
from
0cbe3cb to
536311d
Compare
|
This PR seems to behavior changes even if a config flag is not swapped so I'll leave it open w/o merging for now. |
This is something I did a long time ago, to make date serialization faster and take less space once serialized.
It's not pretty code but it works - I'll leave it up to you to decide if it can be merged or not, or if it needs rework!
Edit: re-pushed with new formatting and tests namespace.