search

mbraceproject / FsPickler

A fast multi-format message serializer for .NET
http://mbraceproject.github.io/FsPickler/
MIT License
326 stars 52 forks source link

Discriminated unions use integer for cases #18

Closed thinkbeforecoding closed 10 years ago

thinkbeforecoding commented 10 years ago

It could be more readable to use case name instead.

thoughts ?

eiriktsarpalis commented 10 years ago

Again, this is an artifact from binary serialization. I should probably do this, but again this will result in performance degradation.

eiriktsarpalis commented 10 years ago

By the way, I would appreciate as much feedback as possible in the format implementations. This is one of the reasons why I published this as a pre-release. Keep them coming!

eiriktsarpalis commented 10 years ago

Fixed in 74c1b29047534ca56a7442383ff3d439e491ea1c

thinkbeforecoding commented 10 years ago

Excellent ! This is Great work.

thinkbeforecoding commented 10 years ago

A small suggestion comes to mind but I'm not sure if it's a good idea anyway:

What if we serialized cases without any values directly as a string.

Like:

type Cases = 
    | Something
    | Another of what: string
type Container = { value: Cases }

You'd get: { value: "Something" } or { value: { Case: "Another", what: "text" }} ...

What do you think ?

eiriktsarpalis commented 10 years ago

This is not possible under the current codebase as it would require explicitly moving union deserialization logic to the format implementation rather than the pickler implementation. Plus, I am not convinced as to how this would improve readability.

By the way, do you have any feedback to give wrt the xml format?

thinkbeforecoding commented 10 years ago

True, It would be a problem with xml...