gsoc: Keeping OsmPlacemarkData synchronized with geometries while editing
Review Request #124570 - Created July 31, 2015 and submitted
In order to achieve this, the following has been done:
Wherever geometries are altered within the annotate plugin, I also modified the osmPlacemarkData objects accordingly
Eg: -a node is deleted =======> a reference within the osmData for that node is removed
-a node is moved =======> the reference of that node within the osmData is changed to the new coordinates
I also realised there's one extra case at writing time i haven't thought of:
Placemarks might have semi-initialized osmData.
Eg. a polygon is loaded from an ".osm" file with complete OsmData. In the editor, a new innerBoundary is added. This new
boundary does not yet have osmData ).
In order to take this case in consideration, I modified the OsmObjectManager to check for such a case.
Tested every case: loading placemarks from files, creating placemarks, modifying loaded placemarks in every way that i thought of. The results are okay.
There are a lot of ways to modify placemarks, I might have skipped a few please report any bugs :)
For the five
*Referencemethod variants here I'd prefer to distinguish them by their names also, not just by the parameter:
containsReference(Coordinates) => containsNodeReference
containsReference(int) => containsMemberReference
removeReference(Coordinates) => removeNodeReference
removeReference(int) => removeMemberReference
changeReference(GeoDataCoordinates,GeoDataCoordinates) => changeNodeReference(GeoDataCoordinates,GeoDataCoordinates)
If you prefer shorter names, we could also do referencesNode, referencesMember, removeNode, removeMember, moveNode.
Ctrl+Shift+R does a quick rename refactoring in QtCreator.
Did you consider moving from the QHash to a QVector (where the keys are implicitly given as (0...vector.size()-1) now that int's are used keys? At least here this would be much easier.
Just wondering, I'm fine with keeping it like this if you prefer.
Shouldn't this one check whether newKey equals oldKey? In that case we'd end up removing both.
Seems odd to use a comma initializer and only initialize one of the variables.
Same here, I'd initialize initialOsmData to nullptr as well.
could osmData be 0 here?
solved mentioned issues
Revision 2 (+251 -55)