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

Be consistent on Tag and Geometry retrieval for OSMContribution #508

Open
SlowMo24 opened this issue Jun 7, 2023 · 3 comments
Open

Be consistent on Tag and Geometry retrieval for OSMContribution #508

SlowMo24 opened this issue Jun 7, 2023 · 3 comments
Labels
priority:low Should be quite a far way down on the agenda question Further information is requested

Comments

@SlowMo24
Copy link
Contributor

SlowMo24 commented Jun 7, 2023

Problem Description

To extract OSHDBTags from an OSMContribution one needs to extract the OSMEntity first like contribution.getEntityBefore().getTags().

The extraction of the Geometry from an OSMContribution is located directly with the OSMContribution e.g. contribution.getGeometryUnclippedBefore().

This seems incoherent and makes client code less readable.

Expected Solution

Move Geometry-retrieving methods from OSMContribution to OSMEntity.

Alternative Solutions

Move OSHDBTags retrieving methods to OSMContribution.

@tyrasd tyrasd added the question Further information is requested label Jun 28, 2023
@tyrasd
Copy link
Member

tyrasd commented Jun 28, 2023

Move Geometry-retrieving methods from OSMContribution to OSMEntity.

This is not really feasible, as the geometry does not only depend on the entity, but also on the timestamp (of the contribution). If the method were to be moved to the OSMEntity class, usage would be even less readable: contribution.getEntityAfter().getGeometryUnclipped(contribution.getTimestamp()), and for the before state, the timestamp would need to be adjusted to a time "a little bit" before the contribution's timestamp, which is not easy to determine as some contributions only "take" a second, while others can take a few seconds (because of slow uploads / big changesets). Currently, this logic is done automatically by the CellIterator internally (see prev variable), and would be hard to replicate outside of that class.

Move OSHDBTags retrieving methods to OSMContribution.

This would be possible, but it would just duplicate an existing method. I don't really see a big benefit of doing so. Would you then also add methods to the OSMContribution class for all other properties of the OSMEntity class as well to be consistent?

@SlowMo24 SlowMo24 added the priority:low Should be quite a far way down on the agenda label Jun 28, 2023
@SlowMo24
Copy link
Contributor Author

this is just a design question so I added the low-priority as it does not affect functionality.

If I understand correctly the problem is the lazy creation of geometries, correct? Otherwise the return from contribution.getEntityAfter() could just be a geometry-aware or time-aware object.

Would you then also add methods to the OSMContribution class for all other properties of the OSMEntity class as well to be consistent?

This is only the second-best solution so I didn' t think about it too long. Tags and geometry seem different from the "meta-info" attributes but you are right. The current state is just as strange as to get meta-info from the entity but tag- and geom-info from the contribution.

@tyrasd
Copy link
Member

tyrasd commented Jun 28, 2023

Ah yes, introducing an intermediate object would indeed be an elegant solution. Then OSMContribution would have:

  • existing methods: getTimestamp(), getContributorUserId(), getChangesetId(), getContributionTypes() / is(…), getOSHEntity()
  • new methods getBefore() and getAfter() which would return objects of a new Class which provides:
    • getGeometry() / getGeometryUnclipped()
    • getEntity()
    • getTags() (which is a shortcut for getEntity().getTags()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:low Should be quite a far way down on the agenda question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants