Skip to content

Make concurrency-friendly by adding a type that captures desired config options. Existing API fully preserved. #68

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

acorello
Copy link

@acorello acorello commented May 4, 2025

Introduced Differ, a struct capturing all the config options (including the one passed as "flag"). It now hosts the implementation of deep.Equal as the method func(Differ)Compare.

Original deep.Equal preserved. Internally it delegates to a new Differ, which is configured from the package-level settings.

Original tests have zero changes ensuring things work as expected.

I'm not confident the naming chosen is consistent / clear. Please help.

I've used Compare instead of Equal as method name on Differ because the Equal is conventionally reserved for fun (Something) Equal(any) bool signature.

I've named the config struct Differ to convey the "thing that will calculate a diff" (even though it's just a wrapper for the config). Perhaps Comparer/Comparison would sound better?

The original Equal returned a []string. I'm returning a named type Delta instead, to encapsulate implementation details and facilitate use/addition of common predicates (eg delta.IsEmpty(), (Delta) Has(diff string) bool, etc.)…
But I read the Type Hiding anti-pattern on your blog. Would you (or the library users) prefer a plain []string instead?

@acorello acorello changed the title Make concurrency-friendly by adding type capturing config. Existing API fully preserved. Make concurrency-friendly by adding a type that captures desired config options. Existing API fully preserved. May 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant