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

[germainc]

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]

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]

@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

[rl-pavel]

Just wanted to bump this up as I have also just ran into a similar issue with NewWhatever methods causing exceptions (in tests, thankfully) because the code-generated methods assume Set: true.