Pass common object to nested structs

I have the following setup

type Foo struct {
    a int
}

func NewFoo(a int) *Foo {
    return &Foo{a: a}
}

type Bar struct {
    foo *Foo
    b bool
}

func NewBar(a int, b bool) *Bar {
    return &Bar{foo: NewFoo(a), b: b}
}

Basically, Foo is the low-level interface to a service and Bar provides a high-level interface on top of it.

Currently, both Foo and Bar are logging using a global logger, which makes it easy to configure it for both of them. However, I want to give users the option to specify their own logger. In any case, it should be the same logger for both types.

I’ve tried a couple of things, but most of them did not feel right. My best solution is the following:

var globalLogger := Logger{}

type Foo struct {
    a int
    logger Logger
}

func NewFoo(a int) *Foo {
    return NewFooWithLogger(a, globalLogger)
}

func NewFooWithLogger(a int, logger Logger) *Foo {
    return &Foo{a: a, logger: logger}
}

type Bar struct {
    foo *Foo
    b bool
    logger Logger
}

func NewBar(a int, b bool) *Bar {
    return NewBarWithLogger(a, b, globalLogger)
}

func NewBarWithLogger(a int, b bool, logger Logger) *Bar {
    return &Bar{foo: NewFooWithLogger(a, logger), b, logger: logger}
}

The reason for the two extra functions is that setting the logger is not the regular use case, hence I don’t want to force regular users to do anything about it.

I’m relatively new to go. Does my approach pass the smell test for more experienced people?

Why not something like this: Go Playground - The Go Programming Language

IMO: If Foo implements a “low level interface” Bar’s constructor should not be constructing it.

1 Like

Hi,

I would consider Functional Options Pattern. On the positive, it allows condensing multiple constructors into a single one, with possibility of extending it later, without breaking client code, and provides clients with sensible defaults (globalLogger in your example). Downside is that the code looks a bit difficult, until you get the hang of it.

First you need to add two options structs for each of your structs, which contain all defaultable options

type FooOptions struct {
	logger Logger
}
...
type BarOptions struct {
	logger Logger
	Foo    *Foo
}

Then, keep only NewFoo and NewBar constructors as follows

// Params before options - 'a' in this case, 
//those without which service cannot work,
// and you, as implementor, cannot know defaults for 
// (for example, an API Key)
// options - for params that can be defaulted (like Logger)
func NewFoo(a int, options ...func(*FooOptions)) *Foo {
	// define defaults
	ops := &FooOptions{}
	ops.logger = globalLogger

	// overwrite configuration with provided options
	for _, option := range options {
		option(ops)
	}

	// configure Foo based on final set of options
	return &Foo{a: a, logger: ops.logger}
}

... 

func NewBar(a int, b bool, options ...func(*BarOptions)) *Bar {
	// define defaults
	ops := &BarOptions{}
	ops.logger = globalLogger
	ops.Foo = NewFoo(a) // Foo With Default Logger

	// overwrite configuration with provided options
	for _, option := range options {
		option(ops)
	}

	// configure Foo based on final set of options
	return &Bar{b: b, logger: ops.logger, foo: ops.Foo}
}

Now, you can define options funcs as follows, for convenience

func FooWithLogger(logger Logger) func(*FooOptions) {
	return func(ops *FooOptions) {
		ops.logger = logger
	}
}

...
func BarWithLogger(logger Logger) func(*BarOptions) {
	return func(ops *BarOptions) {
		ops.logger = logger
	}
}

func BarWithFoo(foo *Foo) func(*BarOptions) {
	return func(ops *BarOptions) {
		ops.Foo = foo
	}
}

And now your client can use it like shown below

func main() {
	// now you can either have bar with default logger, and default Foo
	_ = NewBar(2, true)

	// or provide everything 
	_ = NewBar(2, false, BarWithLogger(Logger{}), BarWithFoo(NewFoo(3, FooWithLogger(Logger{}))))
}

Hope this helps

3 Likes

Thanks for the suggestion. I have two comments:

  1. I don’t like having having a logger Logger argument in the default creation function as in most cases the default will just work fine. Thus, I’d force regular users to always pass nil, making the call more complicated. That might be not too bad right now, but I’d rather not end up with something like NewFoo(3, nil, nil, nil).
  2. NewBar not constructing Foo seems reasonable. My intention was to hide the complexity for regular users. In most cases they don’t need to know and also don’t need to care that there even is the Foo object. Thus, adding it to NewBar makes it more complicated for them without good reason. I’ll have to think about this a little more.

Thanks for the suggestion. I like the idea of having an options struct with default values. What I don’t understand is why I’d take multiple functions to alter the defaults, e.g. options ...func(*BarOptions) instead of just having a new function like NewBarOptions that returns the defaults and push the responsibility to the user to adapt it before calling NewBar.

This would look like

func main() {
    _ = NewBar(2, true)

    o := NewBarOptions()
    o.Logger = Logger{}
    _ = NewBar(2, true, o)
}

My implementation now looks like this. The

c := NewFooConfig()
c.Logger = localLogger

is overkill right now as we only have a single field in the config and could be simplified by

c := FooConfig{Logger: localLogger}

However, this is not future-proof if FooConfig gets more fields that get populated by NewFooConfig.

Other than that I think I’ve incorporated the main points of both @freeformz and @iignat:

  1. NewBar should not construct Foo and rather take it as input.
  2. Neither NewFoo nor NewBar should force the user to supply optional parameters that have sensible defaults, but should have the option to do so.

Thanks!

That’s why the best solution here is to use Functional Options Pattern, as was suggested by @iignat . You can hide default options implementation from user but at the same time allow to configure those if needed. Pattern … in function definition means, that you can either pass those settings or not.

Could you clarify what you are referring to with “that”? As stated above I do indeed see the benefit of optionally passing extra options, but I don’t understand why they should be passed as functions. To me

NewFoo(1, func(ops *FooOptions) {ops.logger = logger})

is more complicated than

ops := FooOptions{logger: logger}
NewFoo(1, ops)
type options {
    logger *log.Logger
}

type optFunc func(*options)

func defaultOptions() *options {
  return &options{
    logger: defaultGlobalLogger,
  }
}

func WithLogger(l *log.Logger) optFunc {
  return func(o *options) {
    o.logger = l
  }
}

type foo struct {
  // something
  logger *log.Logger
}

type Bar struct {
  foo *foo
  data int
  logger *log.Logger
}

func NewBar(data int, opts ... optFunc) *Bar {
  o := defaultOptions()
  for _, fn := range opts {
    fn(o)
  }

  f := &foo{logger: o.logger}
  return &Bar{foo: f, data: data, logger: o.logger}
}

In this case you can implement as many default options as you want with just adding an exported function to allow user configure the behavior. But at the same time, user can simply call NewBar without any optFunc-s to use default options e.g.,

b := NewBar(1) // Default global logger.

// or

b := NewBar(1, WithLogger(myCustomLogger)) // Set custom logger.

Or you can split loggers for foo and Bar e.g.,

type options struct {
  fooLogger *log.Logger
  barLogger *log.Logger
}

func defaultOptions() *options {
  return &options{
    fooLogger: defaultGlobalLogger,
    barLogger: defaultGlobalLogger,
  }
}

func WithFooLogger(l *log.Logger) optFunc {
  return func(o *options) {
    o.fooLogger = l
  }
}

func WithBarLogger(l *log.Logger) optFunc {
  return func(o *options) {
    o.barLogger = l
  }
}

// Now I can do something like this:

b := NewBar(1) // Both foo and Bar use default.
b := NewBar(1, WithFooLogger(myCustomFooLogger)) // foo uses custom and Bar uses global (same can be done with only WithBarLogger).
b := NewBar(1, WithFooLogger(myCustomFooLogger), WithBarLogger(myCustomBarLogger)) // Both foo and Bar use custom loggers.

In the end, you can add new options with default parameters, exposing only an exported function so anyone can use what they need. This hides implementation from the user, allowing more cleaner and flexible ways to customize what they want.

Your example with exposing setup struct for the user is also another way to allow customizable options. But in this case, every time you add an option you need to add checks into your code, that user actually provided something. It is less readable than simple Function options where you already know your defaults, but allow users to customize them.

On the contrary, exposing the options struct to the user you’ll get something like this:
Your package:

type Options struct {
  FooLogger *log.Logger
  BarLogger *log.Logger
}

var defaultOptions = &Options{
  FooLogger: globalLogger,
  BarLogger: globalLogger,
}

type foo struct {
  logger *log.Logger
}

type Bar struct {
  foo *foo
  data int
  logger *log.Logger
}

func NewBar(data int, opts *Options) *Bar {
  if opts == nil {
    return &Bar{
        foo: &foo{logger: defaultOptions.FooLogger},
        data: data,
        logger: defaultOptions.BarLogger,
    }

  // But here you will have to check values for every option which is defined in `Options` struct
  // and use default if it was not set.

  if opts.FooLogger != nil {
    defaultOptions.FooLogger = opts.FooLogger
  }

  if opts.BarLogger != nil {
    defaultOptions.BarLogger = opts.BarLogger
  }

  return &Bar{
        foo: &foo{logger: defaultOptions.FooLogger},
        data: data,
        logger: defaultOptions.BarLogger,
    }
}
  • It looks good with pointers, since you can check them on just nil, but some over parameters will be a headache to track.

And in my code, instead of simply passing functions into constructor I’ll have to do something like:

opts := &Options{
  // Setup what I want.
}

b := NewBar(data, opts)

// Or without options.

b := NewBar(data, nil)

All in all, both ways can take place. It’s just different patterns and you can use what ever fits your goal more.

IMHO, functions look as more flexible and cleaner approach.

Fair. Thanks for the input!

FWIW: I generally suggest passing in loggers to methods via a context since you can build up/curry values into the logger for additional context.