[Marble-devel] Review Request 124553: gsoc: Changes key type for the member hash within OsmPlacemarkData

[Dennis Nienhüser]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124553/#review83246
-----------------------------------------------------------

Ship it!


Looks good.


src/plugins/runner/osm/OsmObjectManager.cpp (line 101)
<https://git.reviewboard.kde.org/r/124553/#comment57480>

    for code readability I'd put the ++index in the previous line, then use index without further modification here.



src/plugins/runner/osm/translators/OsmDocumentTagTranslator.cpp (line 83)
<https://git.reviewboard.kde.org/r/124553/#comment57482>

    I'd move the increment to its own line again



src/plugins/runner/osm/writers/OsmRelationTagWriter.cpp (line 42)
<https://git.reviewboard.kde.org/r/124553/#comment57483>

    ++index (prefer prefix increment)


- Dennis Nienhüser


On July 31, 2015, 3:14 p.m., Marius Stanciu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124553/
> -----------------------------------------------------------
> 
> (Updated July 31, 2015, 3:14 p.m.)
> 
> 
> Review request for Marble.
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> It seems storing data with pointer keys within a QHash wasn't a good idea:
> A polygon's inner boundaries are stored within a QVector, which reallocates it's objects sometimes, thus pointers to inner boundaries are not constant.
> Changed the key to int with the following meaning:
> 
> -1 ===> outer boundary of a polygon
> 0,1,2,3...etc =====> inner boundaries of a polygon in the order provided by polygon->innerBoundaries() ( the QVector order )
> 
> This wasn't a problem until I started modiying inner boundaries in the editor.
> 
> 
> Diffs
> -----
> 
>   src/lib/marble/osm/OsmPlacemarkData.h 58f7e69 
>   src/lib/marble/osm/OsmPlacemarkData.cpp c78888b 
>   src/plugins/runner/osm/OsmObjectManager.cpp 85f78d2 
>   src/plugins/runner/osm/handlers/OsmMemberTagHandler.cpp 2e1c9b7 
>   src/plugins/runner/osm/translators/OsmDocumentTagTranslator.cpp cac7852 
>   src/plugins/runner/osm/writers/OsmRelationTagWriter.cpp 8f79a17 
> 
> Diff: https://git.reviewboard.kde.org/r/124553/diff/
> 
> 
> Testing
> -------
> 
> Usual tests ( added polygons, imported, exported, reimported ), stuff works as expected.
> 
> 
> Thanks,
> 
> Marius Stanciu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20150731/5380729b/attachment-0001.html>