svix / svix-webhooks

The enterprise-ready webhooks service 🦀
https://www.svix.com
MIT License
2.27k stars 156 forks source link

Go SDK: NullableString awkwardness #1198

Open nickstenning opened 7 months ago

nickstenning commented 7 months ago

The way that the Go SDK handles optional strings is somewhat awkward, and while this ship may have sailed, @tasn asked me to provide some feedback here.

First of all, I totally understand that there's a need for a NullableString type of some kind. While idiomatic Go loves to use "zero values" including the empty string, other languages and APIs don't always work well with that, and so a string type that can also represent unset/JSON null is necessary.

But the implementation and use of NullableString in the Go SDK is awkward. Perhaps the most awkward aspect is that the helper function svix.NullableString a) takes a *string rather than a string, and b) returns not an openapi.NullableString but a *openapi.NullableString.

That means that the only way to create an openapi.NullableString from a static string literal in a single line, is the rather unwieldy *svix.NullableString(svix.String("mystring")).

The problem is compounded by the fact that openapi.NullableString is a private type, and as a result it is impossible to write 3rd-party code that manipulates it directly:

// This will not compile because the openapi package is internal.
func nullableString(s string) openapi.NullableString {
    return *svix.NullableString(&s)
}

What's particularly odd about the choice to return a pointer from svix.NullableString is that the data types that need nullable strings only ever want an openapi.NullableString, not a pointer to one, which leads to code like:

appIn := svix.ApplicationIn{
    ...
    Uid: *svix.NullableString(svix.String("myuid"))
}

Suggested alternative

I think a much more Go-friendly implementation here would be to make the type something like database/sql's NullString (https://pkg.go.dev/database/sql#NullString).

package types

type NullableString {
    String string
    Valid bool
}

and then define your helper function as

package svix

func NullableString(s string) types.NullableString {
    return types.NullableString{
        String: s,
        Valid: true,
    }
}

That creates a situation where the zero-valued types.NullableString is null, because the zero value of Valid is false:

type ApplicationIn struct {
    Uid types.NullableString
}

app := ApplicationIn{}

fmt.Printf("%v\n", app.Uid.Valid) // Prints false

And constructing one to pass into a type is equally straightforward:

app := ApplicationIn{
    Uid: NullableString("my-app-id"),
}

fmt.Printf("%v\n", app.Uid.Valid) // Prints true

Here's another example to play around with: https://go.dev/play/p/jYGq7qCCC11

ccqpein commented 3 months ago

How is this issue progressing? It is in working stage or still in proposal?

And I am playing around find this solution can also pass the compiler (but we need to change version in go.mod to > =1.18)

type Nullable[T any] struct {
    V     T
    Valid bool
}

type NullableString Nullable[*string]

func NewNullableString(s *string) NullableString {
    if s == nil {
        return NullableString{V: s, Valid: false}
    }
    return NullableString{V: s, Valid: true}
}