Repository navigation
Second Proposed Solution for Issue 4998 - #5003
Conversation
Modified `AllItemsConstraint`, `ExactCountConstraint`, `NoItemConstraint`, `SomeItemsConstraint` and `PropertyConstraint` to invoke base constraints with the actual value's type instead of object Added `ConstraintCastingTests`
|
@manfred-brands @mikkelbu @stevenaw @Dreamescaper |
|
@LeQuackers Can you fix the build please? |
Slight changes to `Repect.BuildApplyToDelegate(Type)`
Fixed issue where nullable value types would fail in the unit tests because the nullable was cast to either its underlying type or null instead of itself Added `TypeHelper.GetNullableValueTypesFromDeclaredEnumerableInterfaces(Type)`
|
@manfred-brands There was an unexpected issue with nullable value types; they're boxed to either their underlying value or null instead of itself (docs). That ended up adding more complexity than I'd like, but at the moment the unit tests are passing. Any ideas are welcome. |
|
After thinking about it a little more, there could probably be some kind of inner I think that'd clean up the nullable value type issue; I'll experiment with it tomorrow. |
Added `Reflect.InvokeApplyToCollection` method Added `TypeHelper.FindPrimaryEnumerableInterface` method Modified collection constraints to use `ICollectionConstraint`
|
I implemented the idea; let me know if you have any questions/comments/concerns. |
…onDelegate` into associated invoke methods as local static methods
…s type in order to simplify the code a bit
|
@manfred-brands |
Simplify Reflect.ApplyToEnumerable
manfred-brands
left a comment
manfred-brands
left a comment
There was a problem hiding this comment.
I added one commit addressing the naming issue: collection => enumerable
and simplifying the code a bit knowing that non of the parameters can be null.
|
Is this WIP or ready to be merged? |
It is ready, but I wouldn't mind a 2nd review. |
3 participants
This proposal is similar to the first one; however, it's a bit cleaner and is applied to more constraints.
As of writing this, some constraints cast the actual value to an
object, which ends up causing inconsistencies / unexpected behavior when interacting with other constraints, in particularDefaultConstraint.DefaultConstraintwas designed to match BCL behavior, so modifying it has caused a lot of discussion. Although it's a roundabout way of doing it, this proposal maintains theDefaultConstraint'sbehavior and resolves #4998, so it seems like a reasonable compromise.As of writing this the following constraints have been modified:
AllItemsConstraintExactCountConstraintNoItemConstraintPropertyConstraintSomeItemsConstraintIf I missed any, let me know and I'll get them in. Feel free to edit as needed.