Skip to content
This repository has been archived by the owner on May 11, 2022. It is now read-only.

Use functional options instead of global variables #43

Closed
Stebalien opened this issue Feb 21, 2020 · 3 comments · Fixed by #53
Closed

Use functional options instead of global variables #43

Stebalien opened this issue Feb 21, 2020 · 3 comments · Fixed by #53
Labels
kind/enhancement A net-new feature or improvement to an existing feature

Comments

@Stebalien
Copy link
Member

https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis

And remove AutoNATBootDelay, etc...

@Stebalien Stebalien added the kind/enhancement A net-new feature or improvement to an existing feature label Feb 21, 2020
@Stebalien
Copy link
Member Author

cc @willscott

@hsanjuan
Copy link
Contributor

I thought we had come to dislike functional options, particularly for internal APIs.

They are elegant to the outside but are less explicit and introduce an additional redirection: in order to find what an option does I first need to find what the function setting that option does.

It's really annoying with defaults, which are usually not explicit. Some times they are not set, sometimes they are set, sometimes nil can be used to disable something, sometimes it means that the default will be used and taking care of this can happen either somewhere in the code or in the setter function.

I guess part of my problem is that people forget to document things and then one must take the code dive, but it is easier to document fields in an exported struct that to do that for functions that modify fields in a private struct. It is also easier to grep for a struct field in the codebase without having to figure out which struct field, if any, corresponds to the option-function...

Just my two cents...

@Stebalien
Copy link
Member Author

We should document them but I think your other concerns come from exposing the internal options/config struct. We shouldn't do that. The functional options should be the only way to set options.

This is effectively an external API, unfortunately.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants