-
Notifications
You must be signed in to change notification settings - Fork 2
Refactoring to ITypeAdapterConfig - Discussion #2
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
base: ResearchDevelopment
Are you sure you want to change the base?
Conversation
add FrozenTypeAdapterConfig
|
Migrating to ITypeAdapterConfig means updating IMapper to work with a more generic interface, allowing flexibility for different implementations (e.g., FrozenTypeAdapterConfig). For migrating IMapper to ITypeAdapterConfig, we can update ServiceMapper to accept ITypeAdapterConfig instead of TypeAdapterConfig Migrating to ITypeAdapterConfig requires updating the Register method to accept ITypeAdapterConfig, allowing modules to work with any implementation, such as FrozenTypeAdapterConfig.The main challenge is ensuring immutability for frozen configurations. |
|
Your implementation of FrozenTypeAdapterConfig as a decorator is a clean and extensible approach to adding freezing support. The use of ConcurrentDictionary for thread-safety and the flexibility of FrozenTypes and DeepFreeze are excellent additions. I’ve reviewed the code and have a few suggestions to refine the implementation. Prevent Apply and Scan from modifying the config after DeepFreeze or FrozenTypes: Instead of returning a cleared |
It is already ensures not modify.
I'm currently testing the implicit approach to identify additional issues. |
The interface can be simplified to the following form: The remaining methods have been converted to Extensions methods. |
This is because the configuration is mutable until the first call to Adapt (Compile). Frozen config won't bring any improvements.
Because you can embed your insert new rule and bypass the frozen settings. |
|
@DocSvartz You asked me for my opinion on change from a concrete type to an interface. Extracting an interface from an existing concrete type and using the Interface instead of the concrete type is non breaking. Then different concrete types can be instantiated. The added "complexity" is that now you have a factory which needs to know what concrete type you want. The initial factory should default to creating an instance of the type that the interface was extracted from with an optional parameter for creating the FrozenTypeAdapterConfig instead of TypeAdapterConfig. ITypeAdapterConfig Create(bool asFrozen == false) { ..... } Does this answer your question or is it more complicated and I am missing the real question? |
Unfortunately, simple extraction isn't enough here. This is significantly different from the current state of TypeAdapterConfig.
Do you see any other advantages beyond this particular case that this transition to an interface (the ability to create custom configurations) could provide? |
@andrerav @stagep @mokarchi it is basically possible :)
The only remaining questions are about migration to ITypeAdapterConfig for
1.1) Imapper
1.2) IRegister
1.3) Mapster tool
1.4) maybe something else
ITypeAdapterConfig methods count
Regarding the warning This may have already been fixed.
It's either related to concurrent access (I think there was an issue on this topic) or related to dependent types.
I'm sure I've forgotten something else :)