sqids-go icon indicating copy to clipboard operation
sqids-go copied to clipboard

Antipattern of variadic New factory

Open gabizou opened this issue 1 year ago • 1 comments

The New constructor feels misleading from the initial reading, assuming that the options would be consumed as a whole, and not ignored. In many go examples, a variadic argument should be consumed by default.

func New(options ...Options) (*Sqids, error) 

What I'd suggest is a break, but a worthwhile one, where the Options type becomes a function operating on an Option that sequentially get applied (and remains true with the default setting), so that consumers can do something like in this playground (copying here for persistence)

type Option struct {
	Alphabet  string
	MinLength uint8
	Blocklist []string
}

// Options for a custom instance of Sqids
type Options = func(*Option)

// Sqids lets you generate unique IDs from numbers
type Sqids struct {
	alphabet  string
	minLength uint8
	blocklist []string
}

func WithAlphabet(alphabet string) Options {
	return func(o *Option) {
		o.Alphabet = alphabet
	}
}

func WithBlocklist(list []string) Options {
	return func(o *Option) {
		o.Blocklist = list
	}
}

func AddToBlocklist(blocked ...string) Options {
	return func(o *Option) {
		o.Blocklist = append(o.Blocklist, blocked...)
	}
}

func validateOptions(o *Option) (*Option, error) {
	return o, nil
}

// New constructs an instance of Sqids
func New(options ...Options) (*Sqids, error) {
	o := &Option{
		Alphabet:  defaultAlphabet,
		Blocklist: defaultBlocklist,
	}
	for _, opt := range options {
		opt(o)
	}
	o, err := validateOptions(o)
	if err != nil {
		return nil, err
	}

	return &Sqids{
		alphabet:  o.Alphabet,
		minLength: o.MinLength,
		blocklist: o.Blocklist,
	}, nil
}

gabizou avatar Feb 06 '25 06:02 gabizou

@gabizou Thanks for the suggestion. I'll keep this open for now, so we can revisit next time we plan a breaking release.

4kimov avatar Feb 17 '25 15:02 4kimov