opentdf / platform

OpenTDF Platform monorepo enabling the development and integration of _forever control_ of data into new and existing applications. The concept of forever control stems from an increasingly common concept known as zero trust.
BSD 3-Clause Clear License
15 stars 4 forks source link

ADR: Normalize the Functional Options pattern used within the monorepo #283

Open jrschumacher opened 4 months ago

jrschumacher commented 4 months ago

Implementation of the Function Options pattern has diverged

Problem

As the Functional Options pattern has been implemented throughout the monorepo, the pattern is starting to vary. We need to normalize on a pattern so that we can reduce cognitive load.

Ref: https://github.com/opentdf/platform/pull/179#discussion_r1502726661

References

Solution

Options

Functional approach

If you just pass everything by value, you always run on the stack and don’t have to incur the overhead of garbage collection. (The GC will still run by default. But having less on the heap will make the GC have less work to do). ^1

func WithSomething(v string) (func(c Opts) Opts, error) {
    if v == "" {
        return nil, fmt.Errorf("value shouldn't be empty")
    }
    return func(c Opts) Opts {
        c.Value = v
        return c
    }, nil
}

Object oriented / pointer approach

func WithSomethingElse(v string) (func(c *Opts), error) {
    if v == "" {
        return nil, fmt.Errorf("value shouldn't be empty")
    }
    return func(c *Opts) {
        c.Value = v
    }, nil
}
jrschumacher commented 3 months ago

@dmihalcik-virtru @strantalis any feedback on this and can we make a decision?

strantalis commented 2 weeks ago

@jrschumacher I have no strong opinions here. I am open to trying option 1 and if we need to change we can always do that.