Tarmil / FSharp.SystemTextJson

System.Text.Json extensions for F# types
MIT License
329 stars 45 forks source link

Add fluent options builder #141

Closed Tarmil closed 1 year ago

Tarmil commented 1 year ago

This PR introduces a new fluent syntax to define the options of the converter.

// New syntax:
let options =
    JsonFSharpOptions.Default()
        .WithUnionAdjacentTag()
        .WithUnionNamedFields()
        .WithUnwrapOption(false)
        .WithUnionTagName("type")
        .ToJsonSerializerOptions()

// Current syntax equivalent:
let options = JsonSerializerOptions()
JsonFSharpConverter(
    (JsonUnionEncoding.AdjacentTag
    ||| JsonUnionEncoding.NamedFields)
    &&& ~~~JsonUnionEncoding.UnwrapOption,
    unionTagName = "type")
|> options.Converters.Add

New API:

Advantages:

Unresolved questions:

cmeeren commented 1 year ago
  • The fluent syntax looks very C#-ish, even though it works perfectly fine in F#. Should we also add a more F#-ish syntax with module functions?

I'm a bit late to the party, but for the record, I don't think the "F# alternative" adds any value. Fluent syntax works great in F#. Indeed, object programing (using classes etc.) works great and is even recommended in the official F# coding guidelines (not OOO stuff like inheritance, but using interfaces and classes).

Tarmil commented 1 year ago

@cmeeren Yes, I ended up not implementing it.

cmeeren commented 1 year ago

Just to be safe, can you verify that the old-style

options.Converters.Add(
  JsonFSharpConverter(
    unionEncoding = (
      JsonUnionEncoding.Default
      ||| JsonUnionEncoding.ExternalTag
      ||| JsonUnionEncoding.NamedFields
      ||| JsonUnionEncoding.UnwrapFieldlessTags
      ||| JsonUnionEncoding.UnwrapSingleFieldCases
      ||| JsonUnionEncoding.UnionFieldNamesFromTypes),
    unionFieldNamingPolicy = JsonNamingPolicy.CamelCase
  )
)

is equivalent to the new-style

options.Converters.Add(
  JsonFSharpOptions
    .Default()
    .WithUnionExternalTag()
    .WithUnionNamedFields()
    .WithUnionUnwrapFieldlessTags()
    .WithUnionUnwrapSingleFieldCases()
    .WithUnionFieldNamesFromTypes()
    .WithUnionFieldNamingPolicy(JsonNamingPolicy.CamelCase)
  |> JsonFSharpConverter
)

?

Tarmil commented 1 year ago

Yes, it's equivalent. Plus, this:

fsOptions.AddToJsonSerializerOptions(options)

is equivalent to:

JsonFSharpConverter(fsOptions)
|> options.Converters.Add