Skip to content
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

Introduce enum for Photon's address type #517

Merged
merged 3 commits into from
Nov 25, 2020

Conversation

lonvia
Copy link
Collaborator

@lonvia lonvia commented Nov 24, 2020

The enum lists the possible address fields that Photon may index and output. It is also responsible for mapping Nominatim's address rank values to the address type. This should now be the only place where raw Nominatim address rank numbers are used.

PhotonDoc uses the type to define its address in a map instead of having lots of separate fields. That simplifies code in a number
of places.

The commit also disallows adding an address part with the same level as the place object itself. CC @hendrikmoree

Fixes #499.

The enum lists the possible address fields that Photon may index
and output. It is also responsible for mapping Nominatim's address
rank values to the address type.

PhotonDoc uses the type to define its address in a map instead of
having lots of separate fields. That simplifies code in a number
of places.

The commit also disallows adding an address part with the same level
as the place object itself.

Fixes komoot#499.
* @param addressFieldName The name of the address tag to use (without the 'addr:' prefix).
*
* @return 'existingField' potentially with the name field replaced. If existingField was null and
* the address field could be found, then a new map with the address as single entry is returned.
*/
private Map<String, String> extractAddress(Map<String, String> existingField, String addressFieldName) {
private void extractAddress(AddressType addressType, String addressFieldName) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you get addressFieldName also from addressType.getName? Then you could remove it from the parameter list.

BTW: thanks for introducing me to EnumMap - really interesting :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mapping between the GeocodeJSON names and the OSM addr:* tag names is not exactly 1:1. suburb and neighbourhood are already different and we might even add more variants (like 'province' for 'state'). We could add the mapping into the new AddressType enum as an extra field though. Something for an extra PR.

@lonvia lonvia merged commit ced9e71 into komoot:master Nov 25, 2020
@lonvia lonvia deleted the introduce-address-type branch December 5, 2020 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strange results for Dutch provinces
2 participants