matryer / matryer/is

Proposal: value matcher interface in is.Equal

Open
#49 2 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
2k
Forks
67
PR merge metrics
No merged PRs in 30d

Description

I sometimes have cases, where I would like to alter or extend the way `is.Equal` is deciding, if the values are considered equal. After trying several approaches, I came up with the following solution, which does not extend the current API surface, but adds a maximum of flexibility and freedom to the user in regards to how `is.Equal` decides if two inputs are equal.

The proposal is to value a newly defined `matcher` interface, which is defined as follows (the `matcher` interface does not need to become part of the public API of `github.com/matryer/is`, in fact it can just be defined inline where needed):

```golang
type matcher interface{
Match(interface{}) bool
}
```

The actual change, that I am proposing is, to extend the existing [`areEqual`](https://github.com/matryer/is/blob/master/is.go#L245) function like this (lines 9-11):

```golang
// areEqual gets whether a equals b or not.
func areEqual(a, b interface{}) bool {
if isNil(a) && isNil(b) {
return true
}
if isNil(a) || isNil(b) {
return false
}
if matcher, ok := a.(interface{ Match(interface{}) bool }); ok {
return matcher.Match(b)
}
if reflect.DeepEqual(a, b) {
return true
}
aValue := reflect.ValueOf(a)
bValue := reflect.ValueOf(b)
return aValue == bValue
}
```

In theory, this change does alter the working of `is.Equal`. In practice, I assume the chances of this change having negative side effects for users of this package to be extremely low. Only users, that use this package to compare types, that implement this exact interface would be affected. I rate the profit of this change to be way higher than the risk of negative side effects.
If you think, that the current signature (`Match(interface{}) bool`) is too likely to cause problems, the name of the method can easily be altered such that the chance of a collision become negligible (e.g. `MatRyerIsMatch(interface{}) bool` 😜).

If I find acceptance for this proposal, I am happy to provide the necessary PR to update the code, the tests and the documentation.

---

As a side note, the `areEqual` function could be minimally simplified by replacing:

```golang
if isNil(a) && isNil(b) {
return true
}
if isNil(a) || isNil(b) {
return false
}
```

with

```golang
if isNil(a) || isNil(b) {
return isNil(a) && isNil(b)
}
```

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in is.go at the areEqual function referenced by the proposal, then review the existing equality tests. Implement and test the proposed matcher behavior without changing the public matcher API, and update the documentation if the change is accepted.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
testing
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.