[Marble-devel] Review Request 108374: GeoDataLatLonQuad class(with handler and writer) for the KML element gx:LatLonQuad

[Torsten Rahn]

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


Good job! But imho still needs some cleanup and fixes.


src/lib/geodata/data/GeoDataLatLonQuad.cpp
<http://git.reviewboard.kde.org/r/108374/#comment19307>

    I do not think that "longitude1" is a good name. Especially since the specs are pretty specific on the location of each point. So just like with the coords this should imho rather be:
    qreal bottomLeftLatitude(unit) const
    



src/lib/geodata/data/GeoDataLatLonQuad.cpp
<http://git.reviewboard.kde.org/r/108374/#comment19314>

    same with the internal names: Why not just name them
    
    return d->m_bottomLeft.latitude(unit);
    



src/lib/geodata/data/GeoDataLatLonQuad.cpp
<http://git.reviewboard.kde.org/r/108374/#comment19308>

    setBottomLeftLatiude()



src/lib/geodata/data/GeoDataLatLonQuad.cpp
<http://git.reviewboard.kde.org/r/108374/#comment19315>

    see above ...



src/lib/geodata/data/GeoDataLatLonQuad.cpp
<http://git.reviewboard.kde.org/r/108374/#comment19309>

    bottomRightLongitude()



src/lib/geodata/data/GeoDataLatLonQuad.cpp
<http://git.reviewboard.kde.org/r/108374/#comment19310>

    setBottomRightLongitude()



src/lib/geodata/data/GeoDataLatLonQuad.cpp
<http://git.reviewboard.kde.org/r/108374/#comment19311>

    see above



src/lib/geodata/data/GeoDataLatLonQuad.cpp
<http://git.reviewboard.kde.org/r/108374/#comment19312>

    see above



src/lib/geodata/data/GeoDataLatLonQuad.cpp
<http://git.reviewboard.kde.org/r/108374/#comment19313>

    see above ...
    



src/lib/geodata/writers/kml/KmlGroundOverlayWriter.h
<http://git.reviewboard.kde.org/r/108374/#comment19318>

    The content seems to be very private ;-)))



src/lib/geodata/writers/kml/KmlLatLonQuadWriter.cpp
<http://git.reviewboard.kde.org/r/108374/#comment19316>

    What is this code supposed to do?
    I don't get it :-)



tests/TestLatLonQuad.cpp
<http://git.reviewboard.kde.org/r/108374/#comment19317>

    Test looks nice to me :-)


- Torsten Rahn


On Jan. 12, 2013, 11:56 p.m., Mayank Madan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108374/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2013, 11:56 p.m.)
> 
> 
> Review request for Marble, Dennis Nienhüser and Torsten Rahn.
> 
> 
> Description
> -------
> 
> GeoDataLatLonQuad class(with handler and writer) for the KML element gx:LatLonQuad
> 
> 
> Diffs
> -----
> 
>   src/lib/geodata/CMakeLists.txt e3f335d 
>   src/lib/geodata/data/GeoDataLatLonQuad.h PRE-CREATION 
>   src/lib/geodata/data/GeoDataLatLonQuad.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlCoordinatesTagHandler.cpp 884d809 
>   src/lib/geodata/handlers/kml/KmlElementDictionary.h 16da3d3 
>   src/lib/geodata/handlers/kml/KmlElementDictionary.cpp 57d07d3 
>   src/lib/geodata/handlers/kml/KmlLatLonQuadTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlLatLonQuadTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/parser/GeoDataTypes.h 5155826 
>   src/lib/geodata/parser/GeoDataTypes.cpp 63bdbd9 
>   src/lib/geodata/writers/kml/KmlGroundOverlayWriter.h 86e2e9a 
>   src/lib/geodata/writers/kml/KmlLatLonQuadWriter.h PRE-CREATION 
>   src/lib/geodata/writers/kml/KmlLatLonQuadWriter.cpp PRE-CREATION 
>   tests/TestLatLonQuad.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/108374/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mayank Madan
> 
>

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