libp2p/go-libp2p-autonat

Use functional options instead of global variables

Stebalien opened this issue · 3 comments

hsanjuan commented

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 commented

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.

  • 👍1