-
-
Notifications
You must be signed in to change notification settings - Fork 794
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
InvocationShape
should be equality-comparable
#777
Conversation
return false; | ||
} | ||
|
||
if (this.Arguments.Count != other.Arguments.Count) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could probably be just an assertion. The argument count should always be equal if the methods are already the same.
|
||
if (this.partiallyEvaluatedArguments == null) | ||
{ | ||
this.partiallyEvaluatedArguments = PartiallyEvaluateArguments(this.Arguments); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be good to someday think about whether this needs additional precautions for thread-safety. For now I'll pretend to be naïve and think that this operation would yield the same result even when called several times by several threads, so it's fine to just let the last evaluation win.
|
||
if (other.partiallyEvaluatedArguments == null) | ||
{ | ||
other.partiallyEvaluatedArguments = PartiallyEvaluateArguments(other.Arguments); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above.
@@ -21,6 +21,8 @@ protected Setup(InvocationShape expectation) | |||
|
|||
public virtual Condition Condition => null; | |||
|
|||
public InvocationShape Expectation => this.expectation; | |||
|
|||
public LambdaExpression Expression => this.expectation.Expression; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should check whether Expression
, Method
still need to be exposed at all now that they'd be accessible through Expectation
. It might be convenient to keep them around.
@@ -86,7 +86,7 @@ public Setup[] ToArrayLive(Func<Setup, bool> predicate) | |||
// The following verification logic will remember each processed setup so that duplicate setups | |||
// (that is, setups overridden by later setups with an equivalent expression) can be detected. | |||
// To speed up duplicate detection, they are partitioned according to the method they target. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That partitioning might no longer be as crucial as before, since the first equality check Invocation.Equals
performs is for the MethodInfo
s to be equal.
Another refactoring that makes
InvocationShape
that will be needed shortly. It also permits a clearer phrasing inSetupCollection
when a setup overrides another.