Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Builder pattern using functional options #106

Open
jhollowayj opened this issue Sep 30, 2020 · 3 comments
Open

Builder pattern using functional options #106

jhollowayj opened this issue Sep 30, 2020 · 3 comments

Comments

@jhollowayj
Copy link

Right now, it's a little weird to create a backoff that has max tries, and context, and set the delay amount (zero, constant, exponential, never).

What are your thoughts on changing the interface to be something more like this:

backoff.NewBackoff(
    backoff.WithConstantDelay(500 * time.Millisecond),
    backoff.WithContext(ctx), 
    backoff.WithMaxTries(4), 
)

I'm not sure if I would want to put the delay amount as an argument, or maintain the existing constructors for each type.
backoff.WithConstantDelay(...), backoff.WithExponentialDelay(...), backoff.WithZeroDelay(), backoff.WithNoRepeat().
vs
backoff.NewConstantBackoff(...), backoff.NewExponentialBackOff(...), ...

I think this would require rewritting the backoff logic to combine support for context and max tries into the base backoff objects.

Thoughts?

Related reading: https://github.com/tmrts/go-patterns/blob/master/idiom/functional-options.md

@cenkalti
Copy link
Owner

cenkalti commented Oct 1, 2020

Hi @jhollowayj. I don't plan rewriting of this library but Option pattern can be implemented for v5 in future.

@thediveo
Copy link

thediveo commented Jul 19, 2021

@cenkalti could you live with not breaking the API (not even on source level) but with adding the New...WithOpts siblings accepting options instead, similar to what was done with Docker's go-client API?

@cenkalti
Copy link
Owner

Hi @thediveo. The library is in stable state and I don't want to make any changes. v5 probably never going to happen. Sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants