feat: allow configuring `NewDecoder` via callbacks

[lithammer]

Since GH-59 was rejected on the basis that it wasn't useful enough to break the public API. I took a stab at an alternate solution that doesn't break the public API. While still allowing one to configure the encoder and decoder when defining it as a package global (as recommended by the README).

var decoder = NewDecoder(WithIgnoreUnknownKeysDecoderOpt(true))

Also partially solves GH-93.

[zak905]

Maybe worth keeping both functions: the original one with no parameters, and a new one called something like NewDecoderWithOptions.

Concerning the options, why there is an array of functions ? I think one can set all the options no ?

NewDecoder(func(d *Decoder)) {
  d.SetAliasTag("")
  d.ZeroEmpty(true)
   //...etc
}

In this case we will not be gaining much.

I think your approach would helpful, if there is already predefined options ready to use:

var WithAliasTagOpt = func(aliasTag string) func(d *Decoder) {
	return func(d *Decoder) {
		d.SetAliasTag(aliasTag)
	}
}

var WithIgnoreIgnoreUnknownKeysOpt = func(ignoreUnknownKeys bool) func(d *Decoder) {
	return func(d *Decoder) {
		d.IgnoreUnknownKeys(ignoreUnknownKeys)
	}
}


// ..etc an opt for each Setter function

In this case, having something like this would be more elegant:

decoder := NewDecoder(WithAliasTagOpt("alias"), WithIgnoreIgnoreUnknownKeysOpt(false))

Also how about the Encoder struct, we should do the same for NewEncoder ?

Maybe worth having a second opinion.

[lithammer]

Maybe worth keeping both functions: the original one with no parameters, and a new one called something like NewDecoderWithOptions.

Yeah I'm just a bit unsure how to design the API for that to be able to add new options without breaking backwards compatibility 🤔 You could of course apply the same strategy with the callbacks. But I feel like there must be a nicer approach available when you can start from scratch....

Concerning the options, why there is an array of functions ? I think one can set all the options no ?

NewDecoder(func(d *Decoder)) {
  d.SetAliasTag("")
  d.ZeroEmpty(true)
   //...etc
}

In this case we will not be gaining much.

Purely to be able to release it as a non-breaking change since one could still pass in zero arguments to get the old behaviour.

I think your approach would helpful, if there is already predefined options ready to use:

var WithAliasTagOpt = func(aliasTag string) func(d *Decoder) {
	return func(d *Decoder) {
		d.SetAliasTag(aliasTag)
	}
}

var WithIgnoreIgnoreUnknownKeysOpt = func(ignoreUnknownKeys bool) func(d *Decoder) {
	return func(d *Decoder) {
		d.IgnoreUnknownKeys(ignoreUnknownKeys)
	}
}


// ..etc an opt for each Setter function

In this case, having something like this would be more elegant:

decoder := NewDecoder(WithAliasTagOpt("alias"), WithIgnoreIgnoreUnknownKeysOpt(false))

Hmm yeah I guess that would look a bit neater. I guess it could be implemented separately though since the function signature would remain the same.

Also how about the Encoder struct, we should do the same for NewEncoder ?

Most likely! To be honest, I didn't consider it because decoding was my real world use-case.

Maybe worth having a second opinion.

Indeed 🙂

[codecov[bot]]

Codecov Report

Merging #193 (74b3a47) into main (5fca2dc) will increase coverage by 0.09%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #193      +/-   ##
==========================================
+ Coverage   83.23%   83.33%   +0.09%     
==========================================
  Files           4        4              
  Lines         710      714       +4     
==========================================
+ Hits          591      595       +4     
  Misses        103      103              
  Partials       16       16              

Files Changed	Coverage Δ
decoder.go	76.06% <100.00%> (+0.29%)	:arrow_up: