Thank you @hacdias! I wasn't figuring this goism (if it is a goism) out

Reading this bit of code I see: "A func that takes the setting as argument and sets the option and then returns nil."

Can you explain to me if this is in fact idiomatic go and if it is, why sometimes use functional arguments such as https://github.com/ipfs/testground/blob/master/plans/dht/test/create-dht-node.go#L22-L25 and other times do like this?

@daviddias I've used Go in the past but I've never actually seen something like this. As far as I understand, what we have here is a function that takes a setting as argument and sets the option and returns an error if it occurred.

Whoever wrong the code in first place is certainly more capable of explaining why they choose to do it this way than I am. And I'm also curious

@hacdias thank you! @Stebalien could you shed some of wisdom here?

@hacdias this is a pattern we frequently use in our code base https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis. It essentially hacks around Go not supporting optional parameters, by leveraging its support of variadic arguments.

The idea is that you call some function (e.g. ipfs.Unixfs().Add) with the parameters it requires (e.g. ctx), and optionally pass in any additional parameters via functions (e.g. ipfs.Unixfs().Add(ctx, option1, option2, ....). The error that you noticed is part of the function signature and as you can see no error is actually returned (we return nil). However, it's possible that sometimes we might want to return an error. For example, if we want to only set settings.A or settings.B, then if B is set after A we can throw an error. Another example is if inside the option we do some IO operation that might fail, then we can still handle the IO error. Check out that link for more info.

@aschmahmann I get that blog post and I believed I understood it well when I wrote https://github.com/ipfs/testground/blob/master/plans/dht/test/create-dht-node.go#L22-L25. (Let's call this "Way A")

However, the way to get the trickle-dag option to Unixfs is way different in nature (Let's call it "Way B"), is is literally instead of passing the result of a func call (just like https://github.com/ipfs/testground/blob/master/plans/dht/test/create-dht-node.go#L22-L25 and what dave cheney describes) it takes in a function that received the config object to then change it.

Are both ways of doing this part of the same solution? If yes, why use Way B over Way A?

@daviddias I'm not sure if I fully understood your point about the options being "different in nature", but yes they are both part of the same solution.

For "Way A" you used dhtopts.BucketSize(bucketSize). If you look at dhtopts.BucketSize https://github.com/libp2p/go-libp2p-kad-dht/blob/4b7d6fb886de322dd54d7c597e8ab163e087ad74/opts/options.go#L151 you'll see that it's a function that returns a function with the same signature as "Way B". In short this means "Way A" is "Way B" wrapped in another function for convenience and readability.

To make this even more explicit the code here should be replaced with coreopts.Unixfs.Layout(coreopts.TrickleLayout). We actually already have a "Way A" wrapper for the function created here 😄.

I'm not sure if we have an explicitly defined pattern for where/how we put these wrapper "Way A" functions. However, they're generally defined and searchable via code-completion or full-text searching the repo that the options are defined in.