[Marble-devel] Review Request: Code cleanup: Move some static methods from TileLoaderHelper directly to GeoSceneTexture and use TileId as parameter.

[Bernhard Beschow]

> On 2010-04-13 16:42:00, Bastian Holst wrote:
> > trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneTexture.h, line 89
> > <http://reviewboard.kde.org/r/3588/diff/1/?file=23485#file23485line89>
> >
> >     Please add the documentation here.
> >     Perhaps we should change the function name to “themeString” so we don't have an abbreviation here.
> 
> Bernhard Beschow wrote:
>     Hmm, I don't see any real difference between themeStr() and sourceDir(). Perhaps they can be merged? But that should be done by somebody who knows the code very good.
> 
> Bastian Holst wrote:
>     I can find one difference: the abbreviation “Dir” is widely used in Qt and “Str” is not.

By difference I'm referring to what they do. Their only difference is that themeStr() returns "maps/" + sourceDir(). And in a lot of places, there are expressions like "maps/" + sourceDir(), where themeStr() could have been used.


- Bernhard


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


On 2010-04-13 14:59:45, Bernhard Beschow wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3588/
> -----------------------------------------------------------
> 
> (Updated 2010-04-13 14:59:45)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> This patch represents milestone 1 of a series of patches to add more (server-side) storage layouts. In this series of patches, the GeoSceneTexture::StorageLayoutMode enum will be replaced by an abstract class, allowing for easy addition of server-side storage layouts. See [1] for all patches.
> 
> [1] http://github.com/shentok/marble/commits/custom-storage
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdeedu/marble/src/lib/StackedTileLoader.cpp 1114412 
>   trunk/KDE/kdeedu/marble/src/lib/TileLoader.cpp 1114412 
>   trunk/KDE/kdeedu/marble/src/lib/TileLoaderHelper.h 1114412 
>   trunk/KDE/kdeedu/marble/src/lib/TileLoaderHelper.cpp 1114412 
>   trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneTexture.h 1114412 
>   trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneTexture.cpp 1114412 
> 
> Diff: http://reviewboard.kde.org/r/3588/diff
> 
> 
> Testing
> -------
> 
> Works for me, please test.
> 
> 
> Thanks,
> 
> Bernhard
> 
>