deep
deep copied to clipboard
Allow Equal() to be used by different packages in one program.
Some packages may require Equal()'s parameters to be set in a particular way that is incompatible with other users within the same program. The global configuration parameters can be changed and restored, but this could lead to bugs due to race conditions. This commit makes the parameters that control Equal()'s operation part of a structure, Comparer, for which Equal() is now a method. Users can configure their own Comparer struct if desired. To preserve the existing package interface, the package-level Equals() method will use a default Comparer object that relies on pointers to the current global configuration parameters (pointers so that the operation of the global Equals() function will change immediately upon changing the value of any global configuration setting).
Coverage remained the same at 100.0% when pulling 1985ad23777297fb09db0338a8993a89920b4ced on gcla:master into 57af0be209c537ba1d9c2a4b285ab7aea0897e51 on go-test:master.
It's a good idea and would make a good change for v1.1. There's a couple things I'd change wrt the design, etc.
-
s/MakeComparer/New Comparer/
as "NewX" is more idiomatic Go - Struct fields don't need to be pointers, i.e.
FloatPrecision int
is sufficient. Out of curiosity, why are they pointers? - Keeping
Equal(a, b interface{}) []string
is good, really want to do this. I'd simplify and hide the default:
var defaultComparer = Comparer{
// default values
}
Might have more feedback after seeing changes, if you decided to make them (and merge latest to fix the merge conflicts).
Hi - thanks for considering the PR :-)
I'll update the branch so it's based off of latest and resubmit. I'll rename MakeComparer to NewComparer, and hide the default, as you suggest.
For point (2) - it seemed to me that the current implementation made FloatPrecision, MaxDepth, etc a dynamic part of the interface in that each call to Equal()
would use their current values in computing the result. For example, if the user ran
deep.MaxDepth = 5
Equal(a, b)
the result might be different from
deep.MaxDepth = 10
Equal(a, b)
To preserve that behavior, I made the Comparer struct use pointers for these fields, and by default I made the pointers point to these exported variables. So even though, with this PR, Equal()
defers to defaultComparer.Equal()
, these package-level variables continue to have the same effect on the computation.
I'll definitely simplify things if you think that's the best way to go.
Sorry for jumping in, but I was also toying around with this idea for fun. How about something like this? It's backwards compatible without introducing new public types (besides the variadic options). I have this implemented if you're interested.
deep.CompareUnexportedFields = false
deep.FloatPrecision = 1
s1 := s{a: 0.0078125} // 1/128
s2 := s{a: 0.00390625} // 1/256
deep.Equal(s1, s2) // nil
deep.Equal(s1, s2, deep.WithCompareUnexportedFields()) // nil
deep.Equal(s1, s2, deep.WithCompareUnexportedFields(), deep.WithFloatPrecision(3)) // "a: 0.0078125 != 0.00390625"
Hi @flga - your patch would definitely work for my needs, and I'd be happy to just switch over to your implementation :-)