-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
Discussion #3
Comments
Love the initiative. ⚡️
Generally there is quite some refactoring already on the todo list, especially when looking at a possible stable release. As you know, this was my first package doing code-generation stuff, so there are a lot of not-so-best-practices that need cleaning up. I'm really happy about the feedback and any further help in developing. |
Fyi I published a prerelease version I will now close this issue because of inactivity. When you want to discuss a specific point again feel free to open a new issue on it. |
We couldn't find any good json deserialization solution until we found this :) Which solved the biggest problem of generics.
Things I have in mind but of course not sure about them so would love some more insight from you:
1- Merged Generation. (To have one entry file per package):
Having one single file that is generated I think is more tangible for end-user. Friendlier. And you don't to worry about having some entry file for each sub-directory.
2- Making
Mapper
and few other classes included in the package itself and not having to re-generate for each client-side package.2-1- We can achieve that by making Mapper as a singleton that we initialize it with the user side custom mappers, this will lead to having to have to call a method like
initializeMapper()
in main.dart similar to what's injectable doing, the cons are that you have to write one more line, the pros is that you have less code duplicated if you use some kind of modular architecture (given that merged generation is implemented, if not, this will make more sense becauseMapper
can be duplicated in the same package).2-2- By having a singleton we can mock Mapper easily and allow better dependency injection which gives users better testability.
2-3- Any changes to Mapper class won't result in need for re-generating your source code (mostly).
3- Make "_mappers" a Set of
MapperEntry
with generic and BaseMapper instead of a map of type string and aBaseMapper
.3-1- This will allow better readability and more user friendly imo.
4- use https://pub.dev/packages/code_builder instead of string snippets for better formatting and development experience.
5- Naming,
Mapper
is not "is-a" relationship withBaseMapper
instead "has-a" which might confuse some. Can't think of better naming though.6- Maybe divide the generator and the package itself into two separate packages?
So user app size doesn't get affected by builders.
7- Maybe use your awesome package https://pub.dev/packages/type_plus instead of doing it ourselves in this package?
8- Some refactor in general and making the internals more type safe and less dynamic as much as possible.
9- Not quite sure of why "decoder" and "encoder" are nullable functions, it will make it harder to deal with them in case we need to use the BaseMapper directly.
I have experimented a bit with some of these ideas to see how well they might work out, this is the initial draft (didn't try to run tests yet and it's still WIP, although I did a little example locally and basic functionalities seem to work, will work more on it once I have more insight from you and time)
https://github.com/erabti/dart_mappable
What do you think? 😊 @schultek
The text was updated successfully, but these errors were encountered: