ogen-go / ogen

OpenAPI v3 code generator for go
https://ogen.dev
Apache License 2.0
1.27k stars 74 forks source link

Optional Fields: Should NewOptType(v) take a pointer instead? #1274

Open germainc opened 1 month ago

germainc commented 1 month ago

Description

Would it make more sense to instantiate OptNilDateTime (or OptDateTime or any other optional type) by passing in a pointer?

func NewOptNilDateTime(v *time.Time) OptNilDateTime {
    return OptNilDateTime{
        Value: *cmp.Or(v, &time.Time{}),
        Set:   true,
        Null:  v == nil,
    }
}

You would still have the flexibility to write more complex logic when you need it but it would simplify most uses cases.

// System document
type DocumentResponse struct {
    UserID      string
    GeneratedAt time.Time
    SignedAt    *time.Time
    ReviewedAt  *time.Time
    ExpiresAt   *time.Time
}

...

// API document response
type DocumentResponse struct {
    UserID      string         `json:"user_id"`
    GeneratedAt time.Time      `json:"generated_at"`
    SignedAt    OptNilDateTime `json:"signed_at"`
    ReviewedAt  OptNilDateTime `json:"reviewed_at"`
    ExpiresAt   OptNilDateTime `json:"expires_at"`
}

...

// Handler
func (h *Handler) GetDocument(ctx context.Context, params api.GetDocParams) (*api.DocumentResponse, error) {
    doc := h.RetrieveDoc(ctx, params.DocID)

    return &api.DocumentResponse{
        UserID:      doc.UserID,
        GeneratedAt: doc.GeneratedAt,
        SignedAt:    api.NewOptNilDateTime(doc.SignedAt),
        ReviewedAt:  api.NewOptNilDateTime(doc.ReviewedAt),
        ExpiresAt:   api.NewOptNilDateTime(doc.ExpiresAt),
    }, nil
}

...

// Output
{
  "user_id": "2bb516da-db63-4606-85be-d531b6badff5",
  "generated_at": "2024-07-02T22:25:52Z",
  "signed_at": "2024-07-02T23:12:19Z",
  "reviewed_at": null,
  "expires_at": null
}
wildwind123 commented 1 month ago

I use this approach, for nil values.

func TestNilToVal(t *testing.T) {

    var boolean *bool
    resBool := ogencl.NewOptBool(NilToVal(boolean))
    fmt.Println(resBool)
    var str *string
    resString := ogencl.NewOptString(NilToVal(str))
    fmt.Println(resString)

}

func NilToVal[T any](v *T) T {
    if v == nil {
        v = new(T)
    }
    return *v
}
germainc commented 1 month ago

@wildwind123 That approach makes the assumption that nil and the zero value of a type carry the same meaning in your system.

If you do that with a *time.Time it will render out as:

"generated_at": "0001-01-01T00:00:00Z"

which does not have the same meaning as:

"generated_at": null