[Marble-devel] Review Request: Implement what the KML spec says on placemarks regarding MultiGeometry and icons

Torsten Rahn tackat at kde.org
Thu Sep 15 16:48:46 UTC 2011


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102591/#review6539
-----------------------------------------------------------



src/lib/geodata/data/GeoDataPlacemark.h
<http://git.reviewboard.kde.org/r/102591/#comment5784>

    Hm, this looks strange to me and smells of the boolean trap. Why not have a separate
    
    bool hasIconAtCoordinates() const
    
    method?



src/lib/geodata/data/GeoDataPlacemark.cpp
<http://git.reviewboard.kde.org/r/102591/#comment5785>

    btw: we usually prefer preincrement to postincrement whenever it's meant. In case of iterators this good practice can even prevent that you accidently create copies of objects (which might only happen if someone modifies your code later on without thought ...).


- Torsten


On Sept. 12, 2011, 6:03 p.m., Guillaume Martres wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102591/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2011, 6:03 p.m.)
> 
> 
> Review request for Marble and Thibaut Gridel.
> 
> 
> Summary
> -------
> 
> Implement what the KML spec says on placemarks regarding MultiGeometry and icons
> 
> From https://code.google.com/apis/kml/documentation/kmlreference.html#placemark
> "To give the user something to click in the 3D viewer, you would need to
> create a MultiGeometry object that contains both a Point and the other Geometry
> object."
> 
> 
> Diffs
> -----
> 
>   src/lib/geodata/data/GeoDataPlacemark.h 46842d7 
>   src/lib/geodata/data/GeoDataPlacemark.cpp 14634ed 
>   src/lib/layers/PlacemarkLayout.h bc2ceda 
>   src/lib/layers/PlacemarkLayout.cpp 34d7b7f 
> 
> Diff: http://git.reviewboard.kde.org/r/102591/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guillaume
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20110915/bef609c1/attachment.html>


More information about the Marble-devel mailing list