Apply aggregation pattern to data source URLs in services/location #364
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR creates a new util file,
app/utils/data_urls.py, which serves as the root for a new aggregate pattern. For this aggregate, the DataURLs class in the new util file serves as the aggregate root, through which the different URLs to access each data source (which form the boundary) can be retrieved. I made these changes by first creating the new util file, then removing the localBASE_URLvariables that each location service sets and instead referring to the appropriate URL contained within DataURLs in each of the location service files (csbs.py, jhu.py, and nyt.py). It was also necessary to add an import to the new util file within each of these location service files.Why make these changes?
Since each of the three location services "have a" BASE_URL, it seems intuitive that applying an aggregate pattern here would help simplify the domain model, specifically for each of the location services. In the new aggregate, DataURLs is the publicly visible root which each of the location services have a "has a" relationship with, instead of with the individual data source URLs. These URLs instead make up the boundary of the aggregate. Making these data source URLs accessible through enums makes sense, though an alternative would be to store them in a dictionary, as in pull request #363. I chose the name of DataURLs for the aggregate's root because it is fairly descriptive of what the purpose of this aggregate is: to contain the individual URLs to access each data source.