-
Notifications
You must be signed in to change notification settings - Fork 4
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
Separate the fetcher and the parser into two sub-modules #4
Comments
Good idea, what do you think @nandiheath? |
so you mean separate the address-resolver into different sub-modules? the fetcher will only responsible to fetch the ogcio/land department api? |
Yeah that's right. The library as a whole will still stand as one package. Just the fetcher and the result parser is separated into two internal sub-modules/sub-packages as each of them has different testing and maintenance requirements. This would be helpful when, say, a new data source is available or when we experiment with new parsing algorithms. |
@UnKnoWn-Consortium I agree the idea to separate into different modules but the point is the fetcher itself wont do much as it only fetch the api from the data source. It will make more sense if we group the sub-module by data source. For each sub-module it will handle both fetching and parsing, or just fetching, depends on the data source, and most likely returns a list of sorted Address models. So we can create tests that really fit the sub modules. And of course we have to extract the resolver as well. The ultimate goal we could imagine, is the user can provide an option as parameter, which indicate what data sources he will use, and which algo he would like to weight the results. |
@nandiheath I see your point. In that case, each data source will have their own sub-module which is responsible for fetching, parsing, and normalizing the returned values. And then there will be another sub-module for outputting the desirable results based on the normalized list of addresses and user parameters. Let me see if I can start a new branch for such retrofit in these few days. |
Such architecture may make future maintenance and testing slightly easier. What do you think?
The text was updated successfully, but these errors were encountered: