feat: add Cond matcher #60
Conversation
Fn matcher allows to use custom logic for complex matching scenarios
|
@moisesvega @JacobOaks @tchung1118 any feedback (especially on the exposed function name) are more than welcome. |
There was a problem hiding this comment.
@damianopetrungaro @sywhang @tchung1118 what are your thoughts on this vs. just defining your own type that implements Matcher?
I think this could be useful to have when you need to write a lot of different sets of custom matching logic "on-the-fly", but I'm not sure if it's too much to have this level of generalization and the generalization of being able to implement Matcher through a custom type.
gomock/matchers.go
Outdated
| @@ -97,6 +97,18 @@ func (anyMatcher) String() string { | |||
| return "is anything" | |||
| } | |||
|
|
|||
| type fnMatcher struct { | |||
There was a problem hiding this comment.
What are your thoughts on making this type just a redefinition of the func type since it doesn't really need to be a struct?
type fnMatcher fn(any) bool
func (f fnMatcher) Matches(x any) bool {
return f(x)
}
There was a problem hiding this comment.
I am cool with both as I don't see this as a red flag.
I based this implementation on the fact that most of the currently implemented Matcher are structs with one or no fields at all, reducing the implementation difference as much as possible.
|
@JacobOaks, this is not a mutually exclusive option; this can be the building block for the logic you are referring to. As an example, you can leverage the custom function as the building block for the logic you're suggesting: var DefaultUserMatcher = gomock.Fn(func(x any)bool{ _logic here_ })as well as using it on the fly for complex and dynamic matching occurrences. |
|
I think this could be a good convenience feature for this library. |
|
Hey @damianopetrungaro, I think this makes sense to come into the library. Let's do a different name though. Maybe something like |
damianopetrungaro
force-pushed
the
feat_add_fn_matcher
branch
from
749161e
to
6161abd
Compare
|
@JacobOaks makes sense to me! |
There was a problem hiding this comment.
Just one nit about the comment. Otherwise, LGTM!
@tchung1118 can you take a quick look too?
|
@tchung1118 updated the PR message, for the commit message if we squash and merge we can do that via the UI, if not I can rebase and push force the commit with a different message as well, up to you. |
You don't need to rewrite any commit message. When you squash and merge, PR message becomes the commit message of the merge commit by default, so I just wanted the PR message to be the final commit message. |
|
Cool to be merged then @tchung1118 |
Condis a matcher allowing to rely on custom logic for complex matching scenarios.It happened quite a few times that I had to leverage on the
DoAndReturnto perform complex matching use cases, passinggomock.Any()as a matcher.This would have allowed me to reduce a lot of complexity.