[Marble-devel] Review Request: patch for i18n

[Bastian Holst]

> On 2010-06-10 09:20:50, Torsten Rahn wrote:
> > /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFeature.h, line 230
> > <http://reviewboard.kde.org/r/4194/diff/5/?file=28237#file28237line230>
> >
> >     Here you are using a reference for the return type as well. Usually this should be avoided. See:
> >     
> >     http://techbase.kde.org/Policies/Library_Code_Policy#Const_References
> >     
> >     A good reason for a reference would be speed. But I guess for most use cases where speed matters we'd just query an attribute from the ExtendedData stored inside a GeoDataFeature. I guess we just could have a method inside GeoDataFeature which would allow to query the value of a single attribute. This would avoid GeoDataExtendedData copies. So I think we can avoid returning a reference and return by value instead.

Ok valueRef() and value() and addValue() seems to be more reasonable, but not valueRef() and data()


- Bastian


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


On 2010-06-09 12:41:54, hjain wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4194/
> -----------------------------------------------------------
> 
> (Updated 2010-06-09 12:41:54)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> This patch consists of KML handler and geodata classes for ExtendedData where custom data for Marble can be stored. This patch still requires to write code to use this data.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataData.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataData.cpp PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataData_p.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataExtendedData.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataExtendedData.cpp PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataExtendedData_p.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFeature.h 1135207 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFeature.cpp 1135207 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFeature_p.h 1135207 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlDataTagHandler.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlDataTagHandler.cpp PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlDisplayNameTagHandler.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlDisplayNameTagHandler.cpp PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlElementDictionary.h 1135187 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlElementDictionary.cpp 1135187 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlExtendedDataTagHandler.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlExtendedDataTagHandler.cpp PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlValueTagHandler.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlValueTagHandler.cpp PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/parser/GeoDataTypes.h 1135187 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/parser/GeoDataTypes.cpp 1135187 
> 
> Diff: http://reviewboard.kde.org/r/4194/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> hjain
> 
>