Major refactoring of the EquivalencyValidator #1539
Conversation
|
@jnyrup thoughts on this (before I go to far)? |
| public interface IValidateNestedEquivalency | ||
| { | ||
| void RecursivelyAssertEquality(IEquivalencyValidationContext context); | ||
| } |
There was a problem hiding this comment.
I'm not sure I see the use cases or need for specifying Nested and Recursively 🤔
Nested is a part of how we define equivalency.
Recursively is an implementation detail of how we verify equivalency.
There was a problem hiding this comment.
During several of my masterclasses, the students mentioned that it was not obvious that this method would be called recursively. By adding this to the name, it would be enough of a hint to think about the consequences.
There was a problem hiding this comment.
Playing the devil's advocate:
Why do you need to know it's recursive?
If I call a Sort function on sequence I don't know whether it's recursively or iteratively implemented, only that upon return I have a sorted sequence.
brain storming some names:
ValidateEquivalencyValidate(since the Equivalency part is already part of the interface name)
There was a problem hiding this comment.
You need to make sure it's re-entrant. But I fail to come up with a good example of that right now..
|
I meant with the planned improvements in the bullet list ;-) |
|
A rework of
For the remaining I'm not sure what the consequences are, but I have no immediate objects. |
b110fbf to
47973ee
Compare
9e4ef3e to
93206dc
Compare
86509ca to
6c77344
Compare
|
@jnyrup I tried to split the changes into atomic commits, but almost everything affected everything else, at some point I gave up. But if it would help you, I could split them up in commits that won't compile individually. Just let me know. |
Src/FluentAssertions/Collections/GenericCollectionAssertions.cs
Outdated
Show resolved
Hide resolved
6c77344 to
aac544d
Compare
aac544d to
bfbd555
Compare
* Reduce usage of AssertionScope.Current in EquivalencyValidator * Renamed `IEquivalencyValidator` to a more role-based named * Encapsulated `isComplexTypeMap` into `ComplexTypeMap` * Reduce usage of `AssertionScope.Current` in `EquivalencyValidator` * Attach `CyclicReferenceDetector` to the `EquivalencyValidationContext` instead of the current `AssertionScope` and expose `IsCycleReference` through `IEquivalencyValidationContext` * Removed `CanHandle` from `IEquivalencyStep` * Changed the return value of `IEquivalencyStep.Handle` into an enum with values `ContinueWithNext` and `AssertionCompleted` * Split-off the subject and expectation from `IEquivalencyValidationContext` into `Comparands` to emphasize what we're comparing * Renamed `EquivalencyStepCollection` to `EquivalencyPlan` as well as the corresponding `AssertionOptions` property * Added the typed base-class `EquivalencyStep<T>` for convenience * Updated the extensibility documentation as well as the upgrade section
bfbd555 to
7f1dbfa
Compare
After a master-class on maintainable code I recently gave, I realized there's still a lot to improve on the equivalency validation code.
IEquivalencyValidatorto a more role-based namedisComplexTypeMapintoComplexTypeMapAssertionScope.CurrentinEquivalencyValidatorCyclicReferenceDetectorto theEquivalencyValidationContextinstead of the currentAssertionScopeand exposeIsCycleReferencethroughIEquivalencyValidationContextCanHandlefromIEquivalencyStepIEquivalencyStep.Handleinto an enum with valuesContinueWithNextandAssertionCompletedIEquivalencyValidationContextintoComparandsto emphasize what we're comparingEquivalencyStepCollectiontoEquivalencyPlanas well as the correspondingAssertionOptionspropertyEquivalencyStep<T>for convenienceFixes #1534