Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: serialize and deserialize enums #47

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

artem-metliakov
Copy link

Fix the Enums support.
Fix some misspellings.

Now they're saved in FireStore as strings.

This fixes #46

@f-miyu
Copy link
Owner

f-miyu commented Jul 12, 2020

Thank you for your pull request. I'm sorry, but I can't merge this as it is.
Your implement is enum to string but I would rather implement enum to int. However, I want to support enum to string optionally and consider creating ConverterAttribute which is an attribute for specifying a converter to convert a type to other type such as enum to string.

public class Data
{
    [Converter(typeof(EnumStringConverter))]
     TypeEnum Type { get; set; }
}

@artem-metliakov
Copy link
Author

Thank you for your answer. Why do you prefer ints rather than strings?

If I do support for custom ConverterAttribute for converting types to other types: at first the value will be converted to another type by Converter and then will be converted to native type by FieldValueExtensions.ToNativeFieldValue.
Am I get you right?

@jmkyarrow
Copy link

Being able to convert between string and enum would be an absolute dream since that would allow for great compatibility with databases that also interact with typescript projects which use string unions as the equivalent. This couldn't come sooner!

@x2764tech
Copy link

I'd rather have Enums as strings in Firestore, but I can see value in storing enums as numbers.
Some other things to thing about:

  • enums don't have to be ints - they can be any integral type
  • How do you store enums with the Flags attribute? CSV? Arrays of strings or numbers?

Having a Converter for this would allow much broader customisation, but whether these are .Net Converters or a custom converter type (like JSON.NET) is a decision to be made. In my mind, a converter should convert to and from the underlying representation, and the library should expose functionality to handle known cases.

@f-miyu
Copy link
Owner

f-miyu commented Sep 14, 2020

I have updated this library. After all, I decided enum is converted to integer by default because of the fallowing reasons.

  • Enum.ToString method is low performance.
  • I think how enum is converted to string should be customizable. Especially, enum with FlagsAttribute and EnumMemberAttribute should be able to customize what string is converted.

But I have implemented the EnumStringConverter which simply converts enum to string by ToString.

Usage:

public class Data
{
    [DocumentConverter(typeof(EnumStringConverter))]
     TypeEnum Type { get; set; }
}

You can create a custom converter in the same way as EnumStringConverter.

@jmkyarrow
Copy link

Thank you, this is really nice 🔥🔥😋

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enums support
4 participants