search

qgis / qgis4.0_api

Tracker for QGIS 4.0 API related issues and developer discussion
3 stars 1 forks source link

QgsCoordinateReferenceSystem::readXML() return value and const issue #26

Closed SebDieBln closed 8 years ago

SebDieBln commented 8 years ago

Return value

Currently QgsCoordinateReferenceSystem::readXML() always returns true, even in case of an error when a default CRS is constructed. It should return false then and we could get rid of workarounds like this one.

The ugglier - but equally consistent - alternative would be to change the return type to void.

Const issue

The parameter QDomNode & theNode should be made const. As the name suggests, this method reads the XML and does not modify it. It is called in contexts where the node given to this method is const itself and a const_cast<QDomElement&>() hack is needed at the moment.

nyalldawson commented 8 years ago

@sebdiebln is that return value currently used anywhere? If not, I'd say make that change now.

Also note that we only care about maintaining compatibility with the python API, so changes like making arguments or methods const is OK to do without breaking API. So feel free to make the const change in 2.14 if it helps clean up the code :)

SebDieBln commented 8 years ago

is that return value currently used anywhere?

I checked that it is not used in QGIS but it could be used in plugins. OTOH, if it was used somewhere the current behaviour of always returning true would be a bug. So I will change that.

we only care about maintaining compatibility with the python API

Does that imply a C++ plugin must be recompiled and probably rewritten for a minor release, too? What if somebody makes changes to the API that do not affect python (e.g. the method is not available in python) but break C++ plugins? :confused:

nyalldawson commented 8 years ago

That's considered acceptable. We try to strongly discourage use of c++ plugins due to these kinds of issues. It's basically considered that if you write c++ code targeting QGIS, you either need to choose a target version and stick with that or recompile for each new release.

SebDieBln commented 8 years ago

Closing this issue as it does not have to wait for QGIS 3.0. See qgis/QGIS#2569