Mailing List Archive: [RESEND PATCH 08/12] golang/xenlight: add functional options to configure Context

[RESEND PATCH 08/12] golang/xenlight: add functional options to configure Context

May 24, 2021, 1:36 PM

Post #1 of 6 (2 views)

Add a ContextOption type to support functional options in NewContext.
Then, add a variadic ContextOption parameter to NewContext, which allows
callers to specify 0 or more configuration options.

For now, just add the WithLogLevel option so that callers can set the
log level of the Context's xentoollog_logger. Future configuration
options can be created by adding an appropriate field to the
contextOptions struct and creating a With<OptionName> function to return
a ContextOption

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
---
tools/golang/xenlight/xenlight.go | 44 +++++++++++++++++++++++++++++--
1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index f68d7b6e97..65f93abe32 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -136,7 +136,7 @@ func sigchldHandler(ctx *Context) {
}

// NewContext returns a new Context.
-func NewContext() (ctx *Context, err error) {
+func NewContext(opts ...ContextOption) (ctx *Context, err error) {
ctx = &Context{}

defer func() {
@@ -146,8 +146,19 @@ func NewContext() (ctx *Context, err error) {
}
}()

+ // Set the default context options. These fields may
+ // be modified by the provided opts.
+ copts := &contextOptions{
+ logLevel: LogLevelError,
+ }
+
+ for _, opt := range opts {
+ opt.apply(copts)
+ }
+
// Create a logger
- ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
+ ctx.logger = C.xtl_createlogger_stdiostream(C.stderr,
+ C.xentoollog_level(copts.logLevel), 0)

// Allocate a context
ret := C.libxl_ctx_alloc(&ctx.ctx, C.LIBXL_VERSION, 0,
@@ -201,6 +212,35 @@ func (ctx *Context) Close() error {
return nil
}

+type contextOptions struct {
+ logLevel LogLevel
+}
+
+// ContextOption is used to configure options for a Context.
+type ContextOption interface {
+ apply(*contextOptions)
+}
+
+type funcContextOption struct {
+ f func(*contextOptions)
+}
+
+func (fco *funcContextOption) apply(c *contextOptions) {
+ fco.f(c)
+}
+
+func newFuncContextOption(f func(*contextOptions)) *funcContextOption {
+ return &funcContextOption{f}
+}
+
+// WithLogLevel sets the log level for a Context's logger. The default level is
+// LogLevelError.
+func WithLogLevel(level LogLevel) ContextOption {
+ return newFuncContextOption(func(co *contextOptions) {
+ co.logLevel = level
+ })
+}
+
// LogLevel represents an xentoollog_level, and can be used to configre the log
// level of a Context's logger.
type LogLevel int
--
2.17.1

Re: [RESEND PATCH 08/12] golang/xenlight: add functional options to configure Context [ In reply to ]

George.Dunlap at citrix

Jun 18, 2021, 7:44 AM

Post #2 of 6 (2 views)

Permalink

> On May 24, 2021, at 9:36 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
>
> Add a ContextOption type to support functional options in NewContext.
> Then, add a variadic ContextOption parameter to NewContext, which allows
> callers to specify 0 or more configuration options.
>
> For now, just add the WithLogLevel option so that callers can set the
> log level of the Context's xentoollog_logger. Future configuration
> options can be created by adding an appropriate field to the
> contextOptions struct and creating a With<OptionName> function to return
> a ContextOption
>
> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
> ---
> tools/golang/xenlight/xenlight.go | 44 +++++++++++++++++++++++++++++--
> 1 file changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> index f68d7b6e97..65f93abe32 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -136,7 +136,7 @@ func sigchldHandler(ctx *Context) {
> }
>
> // NewContext returns a new Context.
> -func NewContext() (ctx *Context, err error) {
> +func NewContext(opts ...ContextOption) (ctx *Context, err error) {
> ctx = &Context{}
>
> defer func() {
> @@ -146,8 +146,19 @@ func NewContext() (ctx *Context, err error) {
> }
> }()
>
> + // Set the default context options. These fields may
> + // be modified by the provided opts.
> + copts := &contextOptions{
> + logLevel: LogLevelError,
> + }
> +
> + for _, opt := range opts {
> + opt.apply(copts)
> + }
> +
> // Create a logger
> - ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
> + ctx.logger = C.xtl_createlogger_stdiostream(C.stderr,
> + C.xentoollog_level(copts.logLevel), 0)
>
> // Allocate a context
> ret := C.libxl_ctx_alloc(&ctx.ctx, C.LIBXL_VERSION, 0,
> @@ -201,6 +212,35 @@ func (ctx *Context) Close() error {
> return nil
> }
>
> +type contextOptions struct {
> + logLevel LogLevel
> +}
> +
> +// ContextOption is used to configure options for a Context.
> +type ContextOption interface {
> + apply(*contextOptions)
> +}
> +
> +type funcContextOption struct {
> + f func(*contextOptions)
> +}
> +
> +func (fco *funcContextOption) apply(c *contextOptions) {
> + fco.f(c)
> +}

Why all this convolution with interfaces and such, rather than just defining ContextOption as a function pointer? Is it just to keep contextOptions out of the documentation page?

-George

Re: [RESEND PATCH 08/12] golang/xenlight: add functional options to configure Context [ In reply to ]

rosbrookn at gmail

Jun 18, 2021, 8:08 AM

Post #3 of 6 (2 views)

Permalink

On Fri, Jun 18, 2021 at 02:44:15PM +0000, George Dunlap wrote:
>
>
> > On May 24, 2021, at 9:36 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
> >
> > Add a ContextOption type to support functional options in NewContext.
> > Then, add a variadic ContextOption parameter to NewContext, which allows
> > callers to specify 0 or more configuration options.
> >
> > For now, just add the WithLogLevel option so that callers can set the
> > log level of the Context's xentoollog_logger. Future configuration
> > options can be created by adding an appropriate field to the
> > contextOptions struct and creating a With<OptionName> function to return
> > a ContextOption
> >
> > Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
> > ---
> > tools/golang/xenlight/xenlight.go | 44 +++++++++++++++++++++++++++++--
> > 1 file changed, 42 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> > index f68d7b6e97..65f93abe32 100644
> > --- a/tools/golang/xenlight/xenlight.go
> > +++ b/tools/golang/xenlight/xenlight.go
> > @@ -136,7 +136,7 @@ func sigchldHandler(ctx *Context) {
> > }
> >
> > // NewContext returns a new Context.
> > -func NewContext() (ctx *Context, err error) {
> > +func NewContext(opts ...ContextOption) (ctx *Context, err error) {
> > ctx = &Context{}
> >
> > defer func() {
> > @@ -146,8 +146,19 @@ func NewContext() (ctx *Context, err error) {
> > }
> > }()
> >
> > + // Set the default context options. These fields may
> > + // be modified by the provided opts.
> > + copts := &contextOptions{
> > + logLevel: LogLevelError,
> > + }
> > +
> > + for _, opt := range opts {
> > + opt.apply(copts)
> > + }
> > +
> > // Create a logger
> > - ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
> > + ctx.logger = C.xtl_createlogger_stdiostream(C.stderr,
> > + C.xentoollog_level(copts.logLevel), 0)
> >
> > // Allocate a context
> > ret := C.libxl_ctx_alloc(&ctx.ctx, C.LIBXL_VERSION, 0,
> > @@ -201,6 +212,35 @@ func (ctx *Context) Close() error {
> > return nil
> > }
> >
> > +type contextOptions struct {
> > + logLevel LogLevel
> > +}
> > +
> > +// ContextOption is used to configure options for a Context.
> > +type ContextOption interface {
> > + apply(*contextOptions)
> > +}
> > +
> > +type funcContextOption struct {
> > + f func(*contextOptions)
> > +}
> > +
> > +func (fco *funcContextOption) apply(c *contextOptions) {
> > + fco.f(c)
> > +}
>
> Why all this convolution with interfaces and such, rather than just defining ContextOption as a function pointer? Is it just to keep contextOptions out of the documentation page?

Part of the motivation for using functional options is to abstract the
"options" struct, yes. This allows internal defaults to be applied more
easily -- if you require e.g. a ContextOptions struct to be passed by
the caller, how do you know if they intended to override a default, or
if they just didn't set the field? Additionally, using the ContextOption
as an interface allows variadic arguments, which are just convenient for
API users -- the same NewContext function can be used whether you need
to pass 3 options or 0.

The reason we use ContextOption as an interface, rather than function
pointer of sorts is for flexibility in the signatures of ContextOption
implementations. E.g., we could have

func WithLogLevel(lvl LogLevel) ContextOption
func WithLogContext(s string) ContextOption
func WithFooAndBar(s string, n int) ContextOption

See [1] for more background on this pattern.

Thanks,
NR

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

Re: [RESEND PATCH 08/12] golang/xenlight: add functional options to configure Context [ In reply to ]

George.Dunlap at citrix

Jun 18, 2021, 9:18 AM

Post #4 of 6 (2 views)

Permalink

> On Jun 18, 2021, at 4:08 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
>
> On Fri, Jun 18, 2021 at 02:44:15PM +0000, George Dunlap wrote:
>>
>>
>>> On May 24, 2021, at 9:36 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
>>>
>>> Add a ContextOption type to support functional options in NewContext.
>>> Then, add a variadic ContextOption parameter to NewContext, which allows
>>> callers to specify 0 or more configuration options.
>>>
>>> For now, just add the WithLogLevel option so that callers can set the
>>> log level of the Context's xentoollog_logger. Future configuration
>>> options can be created by adding an appropriate field to the
>>> contextOptions struct and creating a With<OptionName> function to return
>>> a ContextOption
>>>
>>> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
>>> ---
>>> tools/golang/xenlight/xenlight.go | 44 +++++++++++++++++++++++++++++--
>>> 1 file changed, 42 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
>>> index f68d7b6e97..65f93abe32 100644
>>> --- a/tools/golang/xenlight/xenlight.go
>>> +++ b/tools/golang/xenlight/xenlight.go
>>> @@ -136,7 +136,7 @@ func sigchldHandler(ctx *Context) {
>>> }
>>>
>>> // NewContext returns a new Context.
>>> -func NewContext() (ctx *Context, err error) {
>>> +func NewContext(opts ...ContextOption) (ctx *Context, err error) {
>>> ctx = &Context{}
>>>
>>> defer func() {
>>> @@ -146,8 +146,19 @@ func NewContext() (ctx *Context, err error) {
>>> }
>>> }()
>>>
>>> + // Set the default context options. These fields may
>>> + // be modified by the provided opts.
>>> + copts := &contextOptions{
>>> + logLevel: LogLevelError,
>>> + }
>>> +
>>> + for _, opt := range opts {
>>> + opt.apply(copts)
>>> + }
>>> +
>>> // Create a logger
>>> - ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
>>> + ctx.logger = C.xtl_createlogger_stdiostream(C.stderr,
>>> + C.xentoollog_level(copts.logLevel), 0)
>>>
>>> // Allocate a context
>>> ret := C.libxl_ctx_alloc(&ctx.ctx, C.LIBXL_VERSION, 0,
>>> @@ -201,6 +212,35 @@ func (ctx *Context) Close() error {
>>> return nil
>>> }
>>>
>>> +type contextOptions struct {
>>> + logLevel LogLevel
>>> +}
>>> +
>>> +// ContextOption is used to configure options for a Context.
>>> +type ContextOption interface {
>>> + apply(*contextOptions)
>>> +}
>>> +
>>> +type funcContextOption struct {
>>> + f func(*contextOptions)
>>> +}
>>> +
>>> +func (fco *funcContextOption) apply(c *contextOptions) {
>>> + fco.f(c)
>>> +}
>>
>> Why all this convolution with interfaces and such, rather than just defining ContextOption as a function pointer? Is it just to keep contextOptions out of the documentation page?
>
> Part of the motivation for using functional options is to abstract the
> "options" struct, yes. This allows internal defaults to be applied more
> easily -- if you require e.g. a ContextOptions struct to be passed by
> the caller, how do you know if they intended to override a default, or
> if they just didn't set the field? Additionally, using the ContextOption
> as an interface allows variadic arguments, which are just convenient for
> API users -- the same NewContext function can be used whether you need
> to pass 3 options or 0.
>
> The reason we use ContextOption as an interface, rather than function
> pointer of sorts is for flexibility in the signatures of ContextOption
> implementations. E.g., we could have
>
> func WithLogLevel(lvl LogLevel) ContextOption
> func WithLogContext(s string) ContextOption
> func WithFooAndBar(s string, n int) ContextOption
>
> See [1] for more background on this pattern.
>
> Thanks,
> NR
>
> [1] https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis

Yes, I frequently use a pattern like the one described in that blog post myself. But that blog post doesn’t use interfaces — the final slide actually has the “option function” type as an open-coded function pointer type.

So my question was, why not do something like this:

type ContextOption func(*contextOptions) error

func WithLogLevel(level LogLevel) ContextOption {
return func(co *contextOptions) {
co.logLevel = level
}
}

ATM the only advantage I can see of defining ContextOption as an interface rather than as a function pointer is that the godoc for ContextOption would look like:

type ContextOption interface {
// contains filtered or unexported fields
}

Rather than

type ContextOption func(*contextOptions) error

Which shows you the name of the unexported field.

Is there another reason I missed?

-George

Re: [RESEND PATCH 08/12] golang/xenlight: add functional options to configure Context [ In reply to ]

rosbrookn at gmail

Jun 18, 2021, 10:00 AM

Post #5 of 6 (2 views)

Permalink

On Fri, Jun 18, 2021 at 04:18:44PM +0000, George Dunlap wrote:
>
>
> > On Jun 18, 2021, at 4:08 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
> >
> > On Fri, Jun 18, 2021 at 02:44:15PM +0000, George Dunlap wrote:
> >>
> >>
> >>> On May 24, 2021, at 9:36 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
> >>>
> >>> Add a ContextOption type to support functional options in NewContext.
> >>> Then, add a variadic ContextOption parameter to NewContext, which allows
> >>> callers to specify 0 or more configuration options.
> >>>
> >>> For now, just add the WithLogLevel option so that callers can set the
> >>> log level of the Context's xentoollog_logger. Future configuration
> >>> options can be created by adding an appropriate field to the
> >>> contextOptions struct and creating a With<OptionName> function to return
> >>> a ContextOption
> >>>
> >>> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
> >>> ---
> >>> tools/golang/xenlight/xenlight.go | 44 +++++++++++++++++++++++++++++--
> >>> 1 file changed, 42 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> >>> index f68d7b6e97..65f93abe32 100644
> >>> --- a/tools/golang/xenlight/xenlight.go
> >>> +++ b/tools/golang/xenlight/xenlight.go
> >>> @@ -136,7 +136,7 @@ func sigchldHandler(ctx *Context) {
> >>> }
> >>>
> >>> // NewContext returns a new Context.
> >>> -func NewContext() (ctx *Context, err error) {
> >>> +func NewContext(opts ...ContextOption) (ctx *Context, err error) {
> >>> ctx = &Context{}
> >>>
> >>> defer func() {
> >>> @@ -146,8 +146,19 @@ func NewContext() (ctx *Context, err error) {
> >>> }
> >>> }()
> >>>
> >>> + // Set the default context options. These fields may
> >>> + // be modified by the provided opts.
> >>> + copts := &contextOptions{
> >>> + logLevel: LogLevelError,
> >>> + }
> >>> +
> >>> + for _, opt := range opts {
> >>> + opt.apply(copts)
> >>> + }
> >>> +
> >>> // Create a logger
> >>> - ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
> >>> + ctx.logger = C.xtl_createlogger_stdiostream(C.stderr,
> >>> + C.xentoollog_level(copts.logLevel), 0)
> >>>
> >>> // Allocate a context
> >>> ret := C.libxl_ctx_alloc(&ctx.ctx, C.LIBXL_VERSION, 0,
> >>> @@ -201,6 +212,35 @@ func (ctx *Context) Close() error {
> >>> return nil
> >>> }
> >>>
> >>> +type contextOptions struct {
> >>> + logLevel LogLevel
> >>> +}
> >>> +
> >>> +// ContextOption is used to configure options for a Context.
> >>> +type ContextOption interface {
> >>> + apply(*contextOptions)
> >>> +}
> >>> +
> >>> +type funcContextOption struct {
> >>> + f func(*contextOptions)
> >>> +}
> >>> +
> >>> +func (fco *funcContextOption) apply(c *contextOptions) {
> >>> + fco.f(c)
> >>> +}
> >>
> >> Why all this convolution with interfaces and such, rather than just defining ContextOption as a function pointer? Is it just to keep contextOptions out of the documentation page?
> >
> > Part of the motivation for using functional options is to abstract the
> > "options" struct, yes. This allows internal defaults to be applied more
> > easily -- if you require e.g. a ContextOptions struct to be passed by
> > the caller, how do you know if they intended to override a default, or
> > if they just didn't set the field? Additionally, using the ContextOption
> > as an interface allows variadic arguments, which are just convenient for
> > API users -- the same NewContext function can be used whether you need
> > to pass 3 options or 0.
> >
> > The reason we use ContextOption as an interface, rather than function
> > pointer of sorts is for flexibility in the signatures of ContextOption
> > implementations. E.g., we could have
> >
> > func WithLogLevel(lvl LogLevel) ContextOption
> > func WithLogContext(s string) ContextOption
> > func WithFooAndBar(s string, n int) ContextOption
> >
> > See [1] for more background on this pattern.
> >
> > Thanks,
> > NR
> >
> > [1] https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis
>
> Yes, I frequently use a pattern like the one described in that blog post myself. But that blog post doesn’t use interfaces — the final slide actually has the “option function” type as an open-coded function pointer type.
>
> So my question was, why not do something like this:
>
> type ContextOption func(*contextOptions) error
>
> func WithLogLevel(level LogLevel) ContextOption {
> return func(co *contextOptions) {
> co.logLevel = level
> }
> }
>
> ATM the only advantage I can see of defining ContextOption as an interface rather than as a function pointer is that the godoc for ContextOption would look like:
>
> type ContextOption interface {
> // contains filtered or unexported fields
> }
>
> Rather than
>
> type ContextOption func(*contextOptions) error
>
> Which shows you the name of the unexported field.
>
> Is there another reason I missed?

Technically it does allow more flexibility in implementing
ContextOption, e.g. you could do...

func (lvl LogLevel) apply(co *contextOptions) { co.logLevel = lvl }

...and then pass a LogLevel directly as a ContextOption. But generally
everyone implements these things as funcs.

I will admit that when it comes to my choice of using the interface
version instead of function pointers, I am just more familiar with the
former and encounter it more often in other Go packages I use.

Thanks,
NR

Re: [RESEND PATCH 08/12] golang/xenlight: add functional options to configure Context [ In reply to ]

George.Dunlap at citrix

Jun 18, 2021, 11:12 AM

Post #6 of 6 (2 views)

Permalink

> On Jun 18, 2021, at 6:00 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
>
> On Fri, Jun 18, 2021 at 04:18:44PM +0000, George Dunlap wrote:
>>
>>
>>> On Jun 18, 2021, at 4:08 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
>>>
>>> On Fri, Jun 18, 2021 at 02:44:15PM +0000, George Dunlap wrote:
>>>>
>>>>
>>>>> On May 24, 2021, at 9:36 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
>>>>>
>>>>> Add a ContextOption type to support functional options in NewContext.
>>>>> Then, add a variadic ContextOption parameter to NewContext, which allows
>>>>> callers to specify 0 or more configuration options.
>>>>>
>>>>> For now, just add the WithLogLevel option so that callers can set the
>>>>> log level of the Context's xentoollog_logger. Future configuration
>>>>> options can be created by adding an appropriate field to the
>>>>> contextOptions struct and creating a With<OptionName> function to return
>>>>> a ContextOption
>>>>>
>>>>> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
>>>>> ---
>>>>> tools/golang/xenlight/xenlight.go | 44 +++++++++++++++++++++++++++++--
>>>>> 1 file changed, 42 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
>>>>> index f68d7b6e97..65f93abe32 100644
>>>>> --- a/tools/golang/xenlight/xenlight.go
>>>>> +++ b/tools/golang/xenlight/xenlight.go
>>>>> @@ -136,7 +136,7 @@ func sigchldHandler(ctx *Context) {
>>>>> }
>>>>>
>>>>> // NewContext returns a new Context.
>>>>> -func NewContext() (ctx *Context, err error) {
>>>>> +func NewContext(opts ...ContextOption) (ctx *Context, err error) {
>>>>> ctx = &Context{}
>>>>>
>>>>> defer func() {
>>>>> @@ -146,8 +146,19 @@ func NewContext() (ctx *Context, err error) {
>>>>> }
>>>>> }()
>>>>>
>>>>> + // Set the default context options. These fields may
>>>>> + // be modified by the provided opts.
>>>>> + copts := &contextOptions{
>>>>> + logLevel: LogLevelError,
>>>>> + }
>>>>> +
>>>>> + for _, opt := range opts {
>>>>> + opt.apply(copts)
>>>>> + }
>>>>> +
>>>>> // Create a logger
>>>>> - ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
>>>>> + ctx.logger = C.xtl_createlogger_stdiostream(C.stderr,
>>>>> + C.xentoollog_level(copts.logLevel), 0)
>>>>>
>>>>> // Allocate a context
>>>>> ret := C.libxl_ctx_alloc(&ctx.ctx, C.LIBXL_VERSION, 0,
>>>>> @@ -201,6 +212,35 @@ func (ctx *Context) Close() error {
>>>>> return nil
>>>>> }
>>>>>
>>>>> +type contextOptions struct {
>>>>> + logLevel LogLevel
>>>>> +}
>>>>> +
>>>>> +// ContextOption is used to configure options for a Context.
>>>>> +type ContextOption interface {
>>>>> + apply(*contextOptions)
>>>>> +}
>>>>> +
>>>>> +type funcContextOption struct {
>>>>> + f func(*contextOptions)
>>>>> +}
>>>>> +
>>>>> +func (fco *funcContextOption) apply(c *contextOptions) {
>>>>> + fco.f(c)
>>>>> +}
>>>>
>>>> Why all this convolution with interfaces and such, rather than just defining ContextOption as a function pointer? Is it just to keep contextOptions out of the documentation page?
>>>
>>> Part of the motivation for using functional options is to abstract the
>>> "options" struct, yes. This allows internal defaults to be applied more
>>> easily -- if you require e.g. a ContextOptions struct to be passed by
>>> the caller, how do you know if they intended to override a default, or
>>> if they just didn't set the field? Additionally, using the ContextOption
>>> as an interface allows variadic arguments, which are just convenient for
>>> API users -- the same NewContext function can be used whether you need
>>> to pass 3 options or 0.
>>>
>>> The reason we use ContextOption as an interface, rather than function
>>> pointer of sorts is for flexibility in the signatures of ContextOption
>>> implementations. E.g., we could have
>>>
>>> func WithLogLevel(lvl LogLevel) ContextOption
>>> func WithLogContext(s string) ContextOption
>>> func WithFooAndBar(s string, n int) ContextOption
>>>
>>> See [1] for more background on this pattern.
>>>
>>> Thanks,
>>> NR
>>>
>>> [1] https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis
>>
>> Yes, I frequently use a pattern like the one described in that blog post myself. But that blog post doesn’t use interfaces — the final slide actually has the “option function” type as an open-coded function pointer type.
>>
>> So my question was, why not do something like this:
>>
>> type ContextOption func(*contextOptions) error
>>
>> func WithLogLevel(level LogLevel) ContextOption {
>> return func(co *contextOptions) {
>> co.logLevel = level
>> }
>> }
>>
>> ATM the only advantage I can see of defining ContextOption as an interface rather than as a function pointer is that the godoc for ContextOption would look like:
>>
>> type ContextOption interface {
>> // contains filtered or unexported fields
>> }
>>
>> Rather than
>>
>> type ContextOption func(*contextOptions) error
>>
>> Which shows you the name of the unexported field.
>>
>> Is there another reason I missed?
>
> Technically it does allow more flexibility in implementing
> ContextOption, e.g. you could do...
>
> func (lvl LogLevel) apply(co *contextOptions) { co.logLevel = lvl }
>
> ...and then pass a LogLevel directly as a ContextOption. But generally
> everyone implements these things as funcs.
>
> I will admit that when it comes to my choice of using the interface
> version instead of function pointers, I am just more familiar with the
> former and encounter it more often in other Go packages I use.

OK. It seems a bit weird to me, but that’s not really a good reason to block it. :-) I just wanted to make sure I understood why it was being chosen.

Acked-by: George Dunlap <george.dunlap@citrix.com>