Mark NodaTime structures as Serializable #226

Closed
GoogleCodeExporter opened this Issue Mar 15, 2015 · 8 comments

Comments

Projects
None yet
2 participants
@GoogleCodeExporter
What situation does this request make simpler?

Currently swapping NodaTime objects in for DateTime and DateTimeOffset, but 
where the latter are marked serializable, the former are not, causing an error 
at serialization boundaries.

Original issue reported on code.google.com by dan...@stackwave.com on 17 Jul 2013 at 1:20

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 15, 2015

Well, this causes an error at binary serialization boundaries. Noda Time 1.2 
will support XML serialization, but I'm not sure about binary serialization. We 
definitely don't want to just add the Serializable attribute - we'd want to 
work out a custom binary serialization format.

That said, I think binary serialization is at least a little better than XML 
serialization in terms of handling immutable structs reasonably cleanly.

I'll have a look at it, but I'm not sure I want to delay 1.2 (which is nearly 
ready) for this.

Original comment by jonathan.skeet on 17 Jul 2013 at 2:37

Well, this causes an error at binary serialization boundaries. Noda Time 1.2 
will support XML serialization, but I'm not sure about binary serialization. We 
definitely don't want to just add the Serializable attribute - we'd want to 
work out a custom binary serialization format.

That said, I think binary serialization is at least a little better than XML 
serialization in terms of handling immutable structs reasonably cleanly.

I'll have a look at it, but I'm not sure I want to delay 1.2 (which is nearly 
ready) for this.

Original comment by jonathan.skeet on 17 Jul 2013 at 2:37

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 15, 2015

Understood, and appreciate the consideration and prompt response.

Apologies, should have been more specific about the serialization issue I'm 
experiencing: I'm serializing to a MemoryStream at a few points, and where the 
serializer was happy with DateTime and DateTime offset, it chokes on the 
NodaTime instances.

Thanks,

- Daniel

Original comment by dan...@stackwave.com on 17 Jul 2013 at 3:07

Understood, and appreciate the consideration and prompt response.

Apologies, should have been more specific about the serialization issue I'm 
experiencing: I'm serializing to a MemoryStream at a few points, and where the 
serializer was happy with DateTime and DateTime offset, it chokes on the 
NodaTime instances.

Thanks,

- Daniel

Original comment by dan...@stackwave.com on 17 Jul 2013 at 3:07

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 15, 2015

I'm assuming this is within BinaryFormatter. I'm having a look now, to see how 
much work it would be.

Note that I would be expecting to support the following types:

- Instant
- Duration
- LocalDateTime
- LocalDate
- LocalTime
- OffsetDateTime
- Period
- ZonedDateTime
- Interval
- Offset

Would that cover everything you need? I would almost certainly *not* serialize 
CalendarSystem or DateTimeZone.

Original comment by jonathan.skeet on 17 Jul 2013 at 3:21

I'm assuming this is within BinaryFormatter. I'm having a look now, to see how 
much work it would be.

Note that I would be expecting to support the following types:

- Instant
- Duration
- LocalDateTime
- LocalDate
- LocalTime
- OffsetDateTime
- Period
- ZonedDateTime
- Interval
- Offset

Would that cover everything you need? I would almost certainly *not* serialize 
CalendarSystem or DateTimeZone.

Original comment by jonathan.skeet on 17 Jul 2013 at 3:21

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 15, 2015

Daniel, this is now fixed in change f172019a3164 - are you able to validate 
that it now works for you?

Original comment by jonathan.skeet on 19 Jul 2013 at 3:36

  • Changed state: Fixed
Daniel, this is now fixed in change f172019a3164 - are you able to validate 
that it now works for you?

Original comment by jonathan.skeet on 19 Jul 2013 at 3:36

  • Changed state: Fixed
@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 15, 2015

Original comment by jonathan.skeet on 19 Jul 2013 at 3:36

  • Added labels: Type-Enhancement, Priority-Medium, Milestone-1.2.0

Original comment by jonathan.skeet on 19 Jul 2013 at 3:36

  • Added labels: Type-Enhancement, Priority-Medium, Milestone-1.2.0
@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 15, 2015

Sorry for the delay. I'm using a subset of the types listed, so that list is 
great for me; I just cloned the latest and am testing now.

Original comment by dan...@stackwave.com on 19 Jul 2013 at 3:49

Sorry for the delay. I'm using a subset of the types listed, so that list is 
great for me; I just cloned the latest and am testing now.

Original comment by dan...@stackwave.com on 19 Jul 2013 at 3:49

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 15, 2015

Works perfectly! Thanks very much,

- Daniel

Original comment by dan...@stackwave.com on 19 Jul 2013 at 4:15

Works perfectly! Thanks very much,

- Daniel

Original comment by dan...@stackwave.com on 19 Jul 2013 at 4:15

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 15, 2015

Great - glad to hear it :) Turned out to be reasonable simple, to be honest. 
Let's just hope we don't need to change the format later ;)

Original comment by jonathan.skeet on 19 Jul 2013 at 4:17

  • Changed state: Verified
Great - glad to hear it :) Turned out to be reasonable simple, to be honest. 
Let's just hope we don't need to change the format later ;)

Original comment by jonathan.skeet on 19 Jul 2013 at 4:17

  • Changed state: Verified

@malcolmr malcolmr modified the milestone: 1.2.0 Mar 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment