avast/retry-go

Proposal: Exported default variables should be constants

Noroth opened this issue · 2 comments

When defining default values, it normally makes no sense to make them mutable. Global default values should either be defined as constants, or be well documented, if there are use cases to globally change the default behavior.

Maybe adding a function in config.go, that creates a new default instance and removing the const block in rety.go would be a better approach here?

func NewWithDefaults() *Config {
	return &Config{
		attempts:      uint(10),
		delay:         100 * time.Millisecond,
		maxJitter:     100 * time.Millisecond,
		onRetry:       func(n uint, err error) {},
		retryIf:       IsRecoverable,
		delayType:     CombineDelay(BackOffDelay, RandomDelay),
		lastErrorOnly: false,
		context:       context.Background(),
	}
}
JaSei commented

Hi, @Noroth, I agree exported variables aren't nice. I don't know why I choose this way. Maybe because to the const isn't possible to set function? Or I don't know, and it doesn't matter. I like your idea of NewWithDefaults, and it looks for me like goish way how to do it. I will do it, or PR is really welcome.

Thanks

Hi @JaSei, I can create a PR later and link it to this issue.