Stop using reflection to map built-in types #3507
Conversation
Just specify them explicitly - simpler and more linker-friendly. Part of npgsql#3300
YohDeadfall
left a comment
YohDeadfall
left a comment
There was a problem hiding this comment.
It's weird that 1074b91 was dropped and that I was requested. What happened?
| TypeHandlerFactory = new DefaultTypeHandlerFactory(typeof(CidrHandler)) | ||
| }.Build()); | ||
|
|
||
| var inetClrTypes = new List<Type> |
There was a problem hiding this comment.
You don't need a list here, arrays, arrays are more compact.
There was a problem hiding this comment.
ReadOnlyIpAddress type is added just below when it's available.
This code gets executed exactly once at startup...
That was my fault - I accidentally got my repos/branches crossed and force-pushed to the wrong place :( Have pushed that commit again. |
|
My (stupid) past self apparently had some other ideas. |
I still see the point in those past ideas, but looking at it now it's about the same amount of code, with the difference that now the code flow is more obvious. |
|
I completely agree. This was triggered by wanting to make Npgsql more linker-friendly, but I really think this also makes it more human-friendly too. |
|
I wonder what you'll think of my next little project :) |
3 participants
Just specify them explicitly - simpler and more linker-friendly.
Part of #3300
Note: there are various other opportunities to refactor the type mapping setup code and make it nicer, but am intentionally restricting this PR to getting rid of the reflection.