Various refactorings and optimizations #494
Conversation
This commit removes the `CurrentInterceptContext` class, which was apparently introduced so that `ExtractProxyCall` and `ExecuteCall` could share data. This need indicates that these two strategies per- haps shouldn't be two separate strategies in the first place; one identifies a setup, and the other then executes it. The solution here is to merge the two interception strategies into one, thereby rendering `CurrentInterceptContext` obsolete. (Even after this, there are cases where interception strategies de- pend on one another; the correct working of Moq relies on them being called in a specific order. Ideally, interception strategies would be totally independent of one another, each one having a distinct res- ponsibility. This might be improved upon at a later time.)
|
|
||
| namespace Moq | ||
| { | ||
| internal sealed class Condition |
There was a problem hiding this comment.
This was previously hidden in a different file.
| @@ -259,31 +259,5 @@ internal class CallInfo | |||
| public MethodInfo Method { get; set; } | |||
| public IEnumerable<Expression> Arguments { get; set; } | |||
| } | |||
|
|
|||
| internal sealed class RemoveMatcherConvertVisitor : ExpressionVisitor | |||
There was a problem hiding this comment.
Dead code, not used anywhere.
| } | ||
| } | ||
|
|
||
| IProxyCall matchedSetup = lastMatchingSetupForSameMethod ?? lastMatchingSetup; |
There was a problem hiding this comment.
matchedSetup essentially takes the place of the redundant CurrentInterceptContext class.
| @@ -101,52 +125,11 @@ private static void ThrowIfReturnValueRequired(IProxyCall call, ICallContext inv | |||
| } | |||
| } | |||
|
|
|||
| internal class ExtractProxyCall : IInterceptStrategy | |||
There was a problem hiding this comment.
Merged with ExtractProxyCall into ExtractAndExecuteProxyCall.
| if (!IsObjectMethod(method)) | ||
| { | ||
| return InterceptionAction.Continue; | ||
| } |
There was a problem hiding this comment.
New fast path. There's no reason why we should perform this check 3 times as we did up until now.
| @@ -340,20 +309,20 @@ public ICallbackResult Callback(Delegate callback) | |||
|
|
|||
| protected virtual void SetCallbackWithoutArguments(Action callback) | |||
| { | |||
| this.setupCallback = delegate { callback(); }; | |||
| this.callbackResponse = (object[] args) => callback(); | |||
There was a problem hiding this comment.
When I first read that code, I was wondering why old-style delegate was used here. Make the lambda parameter explicit so it's clear exactly what is going on here, and also to make this method look more similar to SetCallbackWithArguments (below).
| } | ||
| else | ||
| { | ||
| this.raiseEventResponse = null; |
There was a problem hiding this comment.
This should possibly throw instead, like Callback does when given a null argument.
| } | ||
| else | ||
| { | ||
| this.raiseEventResponse = null; |
There was a problem hiding this comment.
Same here, this should possibly throw instead, like Callback does when given a null argument.
| var targetTypeName = lambda.Parameters[0].Type.Name; | ||
|
|
||
| message.Append(targetTypeName).Append(" ").Append(lambda.ToStringFixed()); | ||
|
|
||
| if (TestMethod != null && FileName != null && FileLine != 0) | ||
| #if !NETCORE |
There was a problem hiding this comment.
Now that the fields are removed when targeting .NET Core, we need this directive to completely omit this piece of code.
| @@ -478,34 +463,57 @@ public string Format() | |||
|
|
|||
| return builder.ToString(); | |||
| } | |||
| } | |||
|
|
|||
| internal class Condition | |||
There was a problem hiding this comment.
Moved into its own file.
No description provided.