qgis / QGIS-Enhancement-Proposals

QEP's (QGIS Enhancement Proposals) are used in the process of creating and discussing new enhancements for QGIS
118 stars 37 forks source link

Rename QgsGeometry methods for clarity #96

Open nyalldawson opened 7 years ago

nyalldawson commented 7 years ago

QGIS Enhancement: Rename QgsGeometry methods for clarity

Date 2017/06/21

Author Nyall Dawson @nyalldawson

Contact nyall dot dawson at gmail dot com

Version QGIS 3.0

Summary

The QgsGeometry class has many methods which perform operations on the geometries, including operations like buffers, reprojecting, transformation, etc.

Currently, there's a confusing mix of methods which alter geometries in place vs those which are const and return a new geometry object. This lack of consistency makes use of the QGIS API difficult. E.g. while

g = feature.geometry()
other_feature.setGeometry(g.buffer(500,8))

works as expected,

g = feature.geometry()
other_feature.setGeometry(g.rotate(45, QgsPointXY(1,2)))

doesn't, because rotate is non-const and performed in place, and the rotate method instead returns an int value.

This can lead to extreme confusion when writing PyQGIS scripts - it's difficult to tell where the error in code like

g = feature.geometry()
g = g.buffer(500, 8 )
g = g.transform( QgsCoordinateTransform( source_crs, dest_crs )
g = g.simplify(5)

is without resorting to studying the api docs (transform returns an int and modifies in place)

Proposed Solution

This could be remedied by renaming all const methods which return a new geometry to use past tense names, e.g.

QgsGeometry::buffer -> QgsGeometry::buffered QgsGeometry::simplify -> QgsGeometry::simplified

The command tense ("transform"/"rotate"/etc) will be reserved for use solely when the operation is performed in place on a geometry.

Affected Files

qgsgeometry.h/.cpp and all files which use the affected methods

Backwards Compatibility

N/A

Votes

(required)

vmora commented 7 years ago

+1, clearer indeed.

I like the const/returning better, but I understand the use of modifying geometries inplace, in this case, shouldn't the function return a ref to this, such that one can write:

feature.geometry().buffer(500, 8).transform(QgsCoordinateTransform(source_crs, dest_crs )).simplify(5)
3nids commented 7 years ago

I guess it'll be easier to detect missing /Out/ annotations, so a big +1 from me!

Le mer. 21 juin 2017 à 09:34, Vincent Mora notifications@github.com a écrit :

+1, clearer indeed.

I like the const/returning better, but I understand the use of modifying geometries inplace, in this case, shouldn't the function return a ref to this, such that one can write:

feature.geometry().buffer(500, 8).transform(QgsCoordinateTransform(source_crs, dest_crs )).simplify(5)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/qgis/QGIS-Enhancement-Proposals/issues/96#issuecomment-309988880, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHxGwyQ0xffoG6Ddi5dvTDExnJLuIOpks5sGMehgaJpZM4OAfZH .

strk commented 7 years ago

On Wed, Jun 21, 2017 at 12:34:57AM -0700, Vincent Mora wrote:

shouldn't the function return a ref to this, such that one can write:

This was my thought as well...