Get Maps by Type #214
Get Maps by Type #214
Conversation
PR Summary
|
src/Gridify/GridifyMapper.cs
Outdated
| yield return map; | ||
| } | ||
| } | ||
| // TODO: need to implement other types that aren't unary expression |
There was a problem hiding this comment.
This needs to be completed to support and test all types; currently, it only works with unary expressions. Adding a couple of additional unit tests for known dotnet types and a few custom types is also required.
There was a problem hiding this comment.
I ppdated the method a bit further, I am not sure what other types of expressions are out there. Where can I find the list?
Probably my lack of depth in understanding expressions. This approach isn't looking so hot anymore, where we have to figure out each expression type to find the property type.
There was a problem hiding this comment.
Handling all types of expressions isn't necessary; I believe these three cover at least 80% of the types, and so far, it looks very promising. Perhaps we should consider adding another test case to determine if we can support custom types. Lastly, the PR seems fine, but the documentation needs to be updated.
| @@ -149,8 +149,31 @@ public void AddMap_DuplicateKey_ShouldThrowErrorIfOverrideIfExistsIsFalse() | |||
| //The thrown exception can be used for even more detailed assertions. | |||
| Assert.Equal("Duplicate Key. the 'test' key already exists", exception.Message); | |||
| } | |||
|
|
|||
| [Theory] | |||
| [InlineData(typeof(DateTime), 0)] | |||
There was a problem hiding this comment.
one improvement would be to add a test case that has a DateTime ... false test is not so useful
There was a problem hiding this comment.
one other thing that I just remembered is nested mappings... does it work with nested ones? Additionally, we should verify mappings that utilize a Select method for collections. It's crucial to conduct further testing prior to proceeding with a merge.
Description
Added a method in mapper to return the maps by type. This can be used to define custom value convertors by the caller.
Discussion - #213
Related issue - #211
Type of change
Checklist