Add an ability to pass options to the constructor

[SaveTheRbtz]

trafficstars

This CR adds functional Options pattern to the Chunker's New and Reset functions[1]. This allows adding new functionality to the Chunker w/o cardinality explosion of the NewWith* and ResetWith* functions.

Currently, option is a private type. Not sure whether there are any benefits of making it public, given that it operates only on fields of the Chunker and latter does not have any public fields (even MinSize/MaxSize are hidden within a private chunkerConfig).

While here, also:

• Deprecated NewWithBoundaries, ResetWithBoundaries, and SetAverageBits function, asking users to migrate to migrate to using options. These could be removed in the new major library release.
  • Not sure about SetAverageBits -- in theory it can be used between chunker uses, though I do not see a usecase that is better than just using Reset or New instead.
• Added a couple of tests, incl. one for the WithBoundaries and WithBuffer.

If options are not used, there is no perf impact on New:

$ benchstat old new                                                     
name          old time/op    new time/op    delta
NewChunker-8    67.0µs ±52%    55.2µs ±20%   ~     (p=0.165 n=10+10)

name          old alloc/op   new alloc/op   delta
NewChunker-8     529kB ± 0%     529kB ± 0%   ~     (all equal)

name          old allocs/op  new allocs/op  delta
NewChunker-8      3.00 ± 0%      3.00 ± 0%   ~     (all equal)

[1] This is "mostly" an API compatible change. Following use-cases will fail to compile (it's unlikely that there are any):

func Foo(io.Reader, chunker.Pol) *chunker.Chunker = chunker.New

golang.org/x/exp/cmd/apidiff output:

$ apidiff old new 
Incompatible changes:
- (*Chunker).Reset: changed from func(io.Reader, Pol) to func(io.Reader, Pol, ...option)
- New: changed from func(io.Reader, Pol) *Chunker to func(io.Reader, Pol, ...option) *Chunker

[fd0]

Whoopsi, wrong button

[SaveTheRbtz]

rebased ontop of a fixed CI.

[MichaelEischer]

I've rebased the PR to fix the conflict with the new BaseChunker struct. As the latter is not released yet, I've just cut down the interface to only provide the option based methods. The only ugly part is that there are now two variants of some options: WithBaseAverageBits / WithAverageBits and WithBaseBoundaries / WithBoundaries.

[SaveTheRbtz]

LGTM! (sadly I can't approve since I'm the PR owner)

[fd0]

I'm fine with the new API, I like function options :)

This is the current api diff:

Incompatible changes:
- (*BaseChunker).Reset: changed from func(Pol) to func(Pol, ...baseOption)
- (*BaseChunker).ResetWithBoundaries: removed
- (*BaseChunker).SetAverageBits: removed
- (*Chunker).Reset: changed from func(io.Reader, Pol) to func(io.Reader, Pol, ...option)
- New: changed from func(io.Reader, Pol) *Chunker to func(io.Reader, Pol, ...option) *Chunker
- NewBase: changed from func(Pol) *BaseChunker to func(Pol, ...baseOption) *BaseChunker
- NewBaseWithBoundaries: removed
Compatible changes:
- WithAverageBits: added
- WithBaseAverageBits: added
- WithBaseBoundaries: added
- WithBoundaries: added
- WithBuffer: added

Are we okay with that? I'm wondering about (*BaseChunker).ResetWithBoundaries and (*BaseChunker).SetAverageBits which got removed...

[MichaelEischer]

Are we okay with that? I'm wondering about (*BaseChunker).ResetWithBoundaries and (*BaseChunker).SetAverageBits which got removed...

These aren't yet included in a release version, so there should be no users. (Except some of my own experimental code ^^ ).