[Marble-devel] Review Request: fixed foreach loops code checker issues

Jens-Michael Hoffmann jmho at jmho.de
Sun Jan 6 21:22:45 UTC 2013


Am Sonntag, 6. Januar 2013, 17:40:11 schrieb Kevin Krammer:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108217/#review24845
> -----------------------------------------------------------
> 
> 
> Consider this review more as a hint for how to handle this in applications
> where the developers care about performance of such loops. Obviously the
> developers here are fine with incurring lookups on each iteration
> otherwise they would not have used foreach on keys() or values() in the
> first place
> 
> 
> src/lib/FileManager.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19068>
> 
>     const_iterator, constBegin
> 
> 
> 
> src/lib/FileManager.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19069>
> 
>     const_iterator, constBegin
> 
> 
> 
> src/lib/FileManager.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19064>
> 
>     no, this is a lookup, the iterator has a value() method that returns
> the value the iterator points to
> 
> 
> 
> src/lib/StackedTileLoader.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19065>
> 
>     const_iterator, constBegin
> 
> 
> 
> src/lib/StackedTileLoader.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19066>
> 
>     const_iterator, constEnd
> 
> 
> 
> src/lib/routing/RouteAnnotator.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19070>
> 
>     const_iterator, constBegin
> 
> 
> 
> src/lib/routing/RouteAnnotator.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19071>
> 
>     const_iterator, constEnd
> 
> 
> 
> src/lib/routing/RouteAnnotator.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19072>
> 
>     no, this is an unneeded lookup
>     itpoint.value() does the same more efficiently
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19073>
> 
>     const_iterator, constBegin
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19074>
> 
>     const_iterator, constEnd
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19075>
> 
>     unneeded lookup overhead, use itpoint.value() instead
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19076>
> 
>     unneeded lookup overhead, use itpoint.value() instead
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19077>
> 
>     unneeded lookup overhead, use itpoint.value() instead
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19078>
> 
>     unneeded lookup overhead, use itpoint.value() instead
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19079>
> 
>     unneeded lookup overhead, use itpoint.value() instead
> 
> 
> 
> tests/RenderPluginTest.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19080>
> 
>     const_iterator, constBegin
> 
> 
> 
> tests/RenderPluginTest.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19081>
> 
>     const_iterator, constEnd
> 
> 
> 
> tests/RenderPluginTest.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19082>
> 
>     unneeded lookup overhead at expected->settings().value(), use
> itpoint.value() instead
> 
> 
> 
> tests/TestGeoDataWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19083>
> 
>     const_iterator, constBegin
> 
> 
> 
> tests/TestGeoDataWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19084>
> 
>     const_iterator, constEnd
> 
> 
> 
> tests/TestGeoSceneWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19085>
> 
>     const_iterator, constBegin
> 
> 
> 
> tests/TestGeoSceneWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19086>
> 
>     const_iterator, constEnd
> 
> 
> 
> tests/TestGeoSceneWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19087>
> 
>     const_iterator, constBegin
> 
> 
> 
> tests/TestGeoSceneWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19088>
> 
>     const_iterator, constEnd
> 
> 
> 
> tests/TestGeoSceneWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19089>
> 
>     const_iterator, constBegin
> 
> 
> 
> tests/TestGeoSceneWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19090>
> 
>     const_iterator, constEnd
> 
> 
> 
> tools/osm-addresses/OsmParser.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19091>
> 
>     const_iterator, constBegin
> 
> 
> 
> tools/osm-addresses/OsmParser.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19092>
> 
>     const_iterator, constEnd
> 
> 
> - Kevin Krammer
> 

Thanks for the thorough review, it is very much appreciated. These are indeed 
important points.


kind regards,
Jens-Michael



More information about the Marble-devel mailing list