[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