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

Cleanup (and rename?) QgsGeometry #28

Open
nyalldawson opened this issue Dec 16, 2015 · 2 comments
Open

Cleanup (and rename?) QgsGeometry #28

nyalldawson opened this issue Dec 16, 2015 · 2 comments

Comments

@nyalldawson
Copy link
Contributor

Since the merge of the new geometry engine, the role of QgsGeometry is undefined. It currently acts as just an implicitly shared container for a QgsAbstractGeometryV2, plus a random bunch of methods for modifying and converting the geometry.

I'd like to see this refined in QGIS 3.0. My thoughts:

  • rename QgsGeometry to QgsGeometryContainer, to better indicate what it's used for and differentiate it from QgsAbstractGeometryV2
  • move all geometry modification (eg buffer, addRing, addPart, ...) and relation methods ( eg contains, crosses, ...) out of this class. Many are already implemented in either QgsGeometryEngine, QgsAbstractGeometryV2 or QgsGeometryUtils and the QgsGeometry versions are just wrappers around these
  • move all the creation methods (eg fromQPointF, ... ) to QgsGeometryFactory (or to their relevant geometry class, eg QgsPointV2::fromQPointF )

What would remain in QgsGeometryContainer would be:

  • methods for accessing and setting the contained QgsAbstractGeometryV2, isEmpty()
  • some conversion routines, eg convertToMultiType, convertToSingleType, segmentize, and a general "bool convertTo( QgsWKBTypes::Type )" and "bool canConvertTo( QgsWKBTypes::Type )"
@raymondnijssen
Copy link

  • Related to this, I think the geometry model should work in an inherited way, so you could do a QgsPoint.buffer() and QgsLine.buffer() without writing the typecasting code to (and from) QgsGeometry all the time.

  • We could merge the QgsPoint and QgsPointXY classes

  • Typecasting to and from QgsGeometry should have more descriptive function names than get() and set().
    (According to the documentation set() will be removed from the python api in qgis4 anyway)

@m-kuhn
Copy link
Member

m-kuhn commented Jun 25, 2019

@raymondnijssen I think parts of this does not require QGIS 4. Mainly because inheritance is not involved in the way your comment suggest.

You can already right now today add a new method

QgsGeometry QgsGeometry::buffer()
{
  .... write some code that calls QgsAbstractGeometry::buffer on the contained geometry.
}

PS: and I am very much in favor of doing this!! This will mean that there is less reason to actually call get() and constGet() which will make the API way easier to use.

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

No branches or pull requests

3 participants