Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add universal Transformer and Comparers #182

Open
dsnet opened this issue Jan 11, 2020 · 0 comments
Open

Add universal Transformer and Comparers #182

dsnet opened this issue Jan 11, 2020 · 0 comments

Comments

@dsnet
Copy link
Member

@dsnet dsnet commented Jan 11, 2020

Perhaps the API for Transformer and Comparer should be expanded to permit functions of following signatures:

Comparer(func(cmp.Path) (eq, ok bool) { ... })
Transformer(func(cmp.Path) (vx, vy reflect.Value, ok bool) { ... })

For Comparer, the eq output reports whether the comparison determined the values to be true. For Transformer, the vx and vy output are the transformed outputs and must both be valid and of the same type. In both cases, the ok bool reports whether the option is applicable.

These two signatures are more complex, but provide a more efficient and natural way for libraries to implement certain options. While a combination of Filters+Comparer or Filters+Transformer can accomplish mostly the same thing, there is oftentimes computed information in the filter that ought to be reused in the comparison or transformation. Combining these together makes implementation both easier and faster.

In the early days of cmp, the signatures above were the original design, but was deemed too complex. While, I think we made the right choice of breaking apart the complex signature into the simpler ones we have today, I'm running into cases where it would be easier to implement with the more complex signature. I imagine the use of this will be limited to libraries like protocmp that are then used by many people.

Most usages of comparers and transformers will just use the simpler signatures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.