Skip to content
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

Simplify logic in ExpressionMatcher.Matches #351

Closed
wants to merge 1 commit into from

Conversation

stakx
Copy link
Contributor

@stakx stakx commented May 31, 2017

This fixes #237.

The method's previous semantics are preserved, but note that it could be simplified to just:

return ExpressionComparer.Default.Equals(this.expression, value as Expression);
//                                                        ^^^^^^^^^^^^^^^^^^^

Nothing in ExpressionMatcher ensures that this.expression cannot be null. Therefore, the null special case could (and for correctness, probably should) be left entirely up to ExpressionComparer.Equals.

@stakx stakx force-pushed the issue-237 branch 2 times, most recently from c373aa8 to ed61f20 Compare June 3, 2017 13:16
@stakx
Copy link
Contributor Author

stakx commented Jun 3, 2017

Closing this PR as I will re-submit its commit as part of a larger PR.

@stakx stakx closed this Jun 3, 2017
@stakx stakx deleted the issue-237 branch June 3, 2017 19:32
@devlooped devlooped locked and limited conversation to collaborators Sep 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compared with null after type conversion using 'as' keyword
1 participant